Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(auth): add authentication endpoints #6265

Merged
merged 83 commits into from
Feb 2, 2024

Conversation

pKorsholm
Copy link
Contributor

@pKorsholm pKorsholm commented Jan 30, 2024

What

  • Add authentication endpoints:
    • /auth/[scope]/[provider]
    • /auth/[scope]/[provider]/callback
  • update authenticate-middleware handler
  • Add scope field to user
  • Add unique constraint on scope and entity_id

note: there's still some remaining work related to jwt auth to be handled, this is mainly focussed on session auth with endpoints

@pKorsholm pKorsholm changed the base branch from feat/customer-store to develop February 1, 2024 01:15
* update datamodel

* update scope handlers

* add index migration

* fix integration tests

* fix integration tests
Copy link
Contributor

@riqwan riqwan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added some comments, lgtm otherwise!

packages/auth/src/providers/email-password.ts Show resolved Hide resolved
packages/auth/src/providers/email-password.ts Outdated Show resolved Hide resolved
packages/auth/src/providers/email-password.ts Show resolved Hide resolved
packages/medusa/src/api-v2/store/customers/route.ts Outdated Show resolved Hide resolved
packages/auth/src/models/auth-user.ts Outdated Show resolved Hide resolved

const { success, error, authUser, location } = authResult
if (location) {
res.redirect(location)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: shouldn't the client be the one doing the redirect?

Copy link
Member

@adrien2p adrien2p left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

few questions :D

const authResult = await service.validateCallback(authProvider, authData)

const { success, error, authUser, location } = authResult
if (location) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if there is a location (the redirect could be configured) then we don't check any success or failure?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, we could validate to ensure that success is required if you think that's appropriate? 😄 just figured that in case of a redirect you would want that no matter what

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in general you have a success redirect and a failure redirect that eventually could have custom query parameters, let see that later

packages/medusa/src/api-v2/store/customers/route.ts Outdated Show resolved Hide resolved
packages/medusa/src/api-v2/store/customers/route.ts Outdated Show resolved Hide resolved
@olivermrbl olivermrbl changed the title Feat/add authentication endpoints for v2 feat(auth): add authentication endpoints Feb 1, 2024
@kodiakhq kodiakhq bot merged commit 9fda6a6 into develop Feb 2, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants