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

server: refactor: introduce AuthUser interface #838

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

paul-marechal
Copy link
Member

Replace the references to OAuth2User by AuthUser. This allows downstream extenders to more easily contribute alternative OAuth2 providers: If the expected data is stored in different attributes it will be possible to bridge it by implementing the proper AuthUser.

@paul-marechal
Copy link
Member Author

Note that there seems to be an issue with formatting in this project...

I'm using VS Code along its Java extension to develop and it often feels like reorganizing or cleaning up imports.

I'd be in favor of keeping the cleanup as part of these changes.

It would be good to look into making auto-formatting part of the build step, maybe as a gradle plugin of some kind?

Replace the references to `OAuth2User` by `AuthUser`. This allows
downstream extenders to more easily contribute alternative OAuth2
providers: If the expected data is stored in different attributes it
will be possible to bridge it by implementing the proper `AuthUser`.
@paul-marechal
Copy link
Member Author

Rebased my branch.

remove dependency on OAuth2User from AuthUser by introducing an
AuthUserFactory service. This should allow extenders to more easily
customize attribute access when creating AuthUser instances.
Comment on lines +6 to +20
@Service
public class AuthUserFactory {

public AuthUser createAuthUser(String providerId, OAuth2User oauth2User) {
return new DefaultAuthUser(
oauth2User.getName(),
oauth2User.getAttribute("avatar_url"),
oauth2User.getAttribute("email"),
oauth2User.getAttribute("name"),
oauth2User.getAttribute("login"),
providerId,
oauth2User.getAttribute("html_url")
);
}
}
Copy link
Member Author

Choose a reason for hiding this comment

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

@HeikoBoettger this class should make it easier to customize AuthUser instances. Right now it is hardcoded but you can later make this factory depend on some Spring configuration to customize attribute access.

Choose a reason for hiding this comment

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

@paul-marechal I am not so familiar with spring, do you know how to map the "login" in the spring configuration? I only know about the "spring.security.oauth2.client.provider.my-oauth-provider.user-name-attribute=name". May be short hint somewhere in the openvsx documentation on how to customize the attribute access would be good.

Great work by the way.

@amvanbaren
Copy link
Contributor

@HeikoBoettger Can you share which additional identity provider you were able to implement using this PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

3 participants