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

Add flag to authenticate with OIDC mode #57

Merged
merged 6 commits into from
Dec 27, 2016

Conversation

lbalmaceda
Copy link
Contributor

@lbalmaceda lbalmaceda commented Dec 21, 2016

Make legacy authentication methods use /oauth/token endpoint if client.setOAuth2Preferred(true) is called. The methods that change it's behavior with the flag are:

  • login(String, String, String) --> calls login(String, String) and sets the connection as realm.
  • tokenInfo(String) --> calls userInfo(String)
  • signUp(String, String, String) --> replaces the login call with login(String, String) and sets the connection as realm.
  • signUp(String, String, String, String) --> replaces the login call with login(String, String) and sets the connection as realm.

This PR depends on #56 as there was no way to set a realm value before that.

@@ -421,6 +450,10 @@ public AuthenticationRequest loginWithEmail(@NonNull String email, @NonNull Stri
@SuppressWarnings("WeakerAccess")
@Deprecated
public Request<UserProfile, AuthenticationException> tokenInfo(@NonNull String idToken) {
Copy link
Member

Choose a reason for hiding this comment

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

This is wrong, userInfo is meant for access_token

@@ -191,6 +212,12 @@ public void setUserAgent(String userAgent) {
*/
@SuppressWarnings("WeakerAccess")
public AuthenticationRequest login(@NonNull String usernameOrEmail, @NonNull String password, @NonNull String connection) {
Copy link
Member

Choose a reason for hiding this comment

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

What I'd do is to chose the grant type and legacy endpoint based on the pipeline flag. If it's off, ro and connection is used otherwise token and realm

@@ -223,7 +250,7 @@ public AuthenticationRequest login(@NonNull String usernameOrEmail, @NonNull Str
Map<String, Object> requestParameters = ParameterBuilder.newBuilder()
Copy link
Member

Choose a reason for hiding this comment

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

I'd keep this as it was, using password grant, since realm grant without a realm is pretty useless. We can recommend using the other method and even deprecating this or explaining the default directory restriction of password grant

@lbalmaceda lbalmaceda force-pushed the new-pipeline-on-legacy-methods branch 3 times, most recently from f8e2c16 to f281ef4 Compare December 22, 2016 22:12
@hzalaz hzalaz removed their assignment Dec 26, 2016
@hzalaz hzalaz force-pushed the new-pipeline-on-legacy-methods branch from f281ef4 to 792be5d Compare December 27, 2016 13:42
@hzalaz hzalaz force-pushed the new-pipeline-on-legacy-methods branch from 792be5d to b768025 Compare December 27, 2016 14:04
@hzalaz hzalaz added this to the 1.4.0 milestone Dec 27, 2016
@hzalaz hzalaz changed the title Add a flag to use /oauth/token for login in legacy methods. Add flag to authenticate with OIDC mode Dec 27, 2016
@hzalaz hzalaz merged commit f9195a1 into master Dec 27, 2016
@hzalaz hzalaz deleted the new-pipeline-on-legacy-methods branch December 27, 2016 21:28
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.

2 participants