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

Thomas/check email verification #441

Merged
merged 1 commit into from
Jan 19, 2024

Conversation

balzdur
Copy link
Contributor

@balzdur balzdur commented Jan 16, 2024

In this PR :

  • add email verified check for the password sign in provider
  • delete duplicate Firebase implementation

Linear Issue

Comment on lines +9 to +10
type FireBaseTokenRepository interface {
VerifyFirebaseToken(ctx context.Context, firebaseToken string) (models.FirebaseIdentity, error)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only declare the interface of what is expected to be injected here

Copy link
Contributor

Choose a reason for hiding this comment

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

let's have a look at this one tomorrow if you want : it's probably not the right place to put this

FirebaseTokenRepository: FireBaseTokenRepository{
firebaseClient: firebaseClient,
},
FirebaseTokenRepository: firebase.New(firebaseClient),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Inject an instance of the already factorised firebase repository

@balzdur balzdur changed the base branch from master to thomas/unknwon-user-error January 16, 2024 17:06
Comment on lines +9 to +10
type FireBaseTokenRepository interface {
VerifyFirebaseToken(ctx context.Context, firebaseToken string) (models.FirebaseIdentity, error)
Copy link
Contributor

Choose a reason for hiding this comment

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

let's have a look at this one tomorrow if you want : it's probably not the right place to put this

}
return c.verifier.VerifySessionCookie(ctx, firebaseToken)
if token.Firebase.SignInProvider == "password" && token.Claims["email_verified"] == false {
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

@Pascal-Delange Pascal-Delange left a comment

Choose a reason for hiding this comment

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

sorry, j'avais oublié de valider

@balzdur balzdur merged commit c7d1750 into thomas/unknwon-user-error Jan 19, 2024
2 checks passed
@balzdur balzdur deleted the thomas/check-email-verification branch January 19, 2024 09:28
balzdur added a commit that referenced this pull request Jan 19, 2024
balzdur added a commit that referenced this pull request Jan 19, 2024
* feat(auth): return error message when user is unknown

* feat(auth): add email verification check (#441)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants