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

Don't allow registration with OAuth2 #341

Closed
Bo0mer opened this issue Jun 29, 2022 · 1 comment
Closed

Don't allow registration with OAuth2 #341

Bo0mer opened this issue Jun 29, 2022 · 1 comment

Comments

@Bo0mer
Copy link

Bo0mer commented Jun 29, 2022

Hi,

First of all thanks for your awesome work!

I have use case where I wan't to allow sign in with OAuth2 for existing application users, but I don't want to allow new registrations (the registration process happens manually by an administrator).
However, I'm not sure how I can achieve that. I've looked at the code of the OAuth2 module and I noticed that there is no event fired before attempting to save the user into the OAuth2ServerStorer:

authboss/oauth2/oauth2.go

Lines 235 to 249 in e74112f

user, err := storer.NewFromOAuth2(r.Context(), provider, details)
if err != nil {
return errors.Wrap(err, "failed to create oauth2 user from values")
}
user.PutOAuth2Provider(provider)
user.PutOAuth2AccessToken(token.AccessToken)
user.PutOAuth2Expiry(token.Expiry)
if len(token.RefreshToken) != 0 {
user.PutOAuth2RefreshToken(token.RefreshToken)
}
if err := storer.SaveOAuth2(r.Context(), user); err != nil {
return err
}

The only solution I see is to implement my own OAuth2 module that fires an event prior to saving the user. In the event handler I'll check whether the user exists, and if it does not, I'll stop the execution of the OAuth2 module (effectively skip saving user into the store).

Other option I see is to have a custom OAuth2 module that handles specific error returned from the OAuth2ServerStorer.SaveOAuth2 and again skip persisting the user.

I like the first approach better, but still I wanted to hear whether this is something that anyone else has considered and if it will eventually fit into authboss itself.

Best,
Ivan

@aarondl
Copy link
Member

aarondl commented Mar 13, 2023

Hi Ivan, I haven't considered this one before. The event seems a bit strange as it's a sort of bail out event which is more rare.

It's considered a fine practice to fork these modules, they don't change much at this time so I wouldn't hesitate.

@aarondl aarondl closed this as completed Mar 13, 2023
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

No branches or pull requests

2 participants