davido / gerrit-oauth-provider

OAuth2 authentication provider for Gerrit Code Review. Please upload changes for review to: https://gerrit-review.googlesource.com/#/admin/projects/plugins/oauth
Apache License 2.0
141 stars 84 forks source link

Use json extractor CAS 6.x oauth responses for access tokens #129

Closed caowenbo closed 2 years ago

caowenbo commented 5 years ago

At CAS V6.0.x the OAuth responses for access tokens is produced as JSON.

davido commented 5 years ago

Thanks for your contribution. Can you please upload a change to this gerrit instance: [1]?

[1] https://gerrit-review.googlesource.com/admin/repos/plugins/oauth

caowenbo commented 5 years ago

OK,I will.

piotrekfus91 commented 5 years ago

Hi, how about finishing this issue? We need it as well.

davido commented 5 years ago

The change is under review already, it would be great if someone could test it and report verified bit: [1].

[1] https://gerrit-review.googlesource.com/c/plugins/oauth/+/234973

piotrekfus91 commented 5 years ago

We tested @caowenbo change. While changing token extractor is correct, indeed,what is required as well is changing interpretation of user profile response from CAS. The plugin expects list, while an object is returned: https://github.com/davido/gerrit-oauth-provider/blob/master/src/main/java/com/googlesource/gerrit/plugins/oauth/CasOAuthService.java#L113 I couldn't manage to bind CAS accounts into existing ones, even using fix-legacy-user-id = true.

davido commented 5 years ago

Thanks for testing. Can you paste (obfuscated) result returned from CAS, it would be easier to change the parsing code. Or could you contribute the missing change?

piotrekfus91 commented 5 years ago

Response from CAS:

  "service": "XXX",
  "attributes": {
    "authenticationDate": "2019-08-28T08:27:17.395282Z[Etc/UTC]",
    "authenticationMethod": "LdapAuthenticationHandler",
    "credentialType": "UsernamePasswordCredential",
    "email": "XXX",
    "isFromNewLogin": "true",
    "login": "XXX",
    "longTermAuthenticationRequestTokenUsed": "false",
    "name": "XXX",
    "successfulAuthenticationHandlers": "LdapAuthenticationHandler"
  },
  "id": "XXX",
  "client_id": "gerrit"
}

CAS configuration:

cas.authn.attributeRepository.ldap[0].attributes.cn=name
cas.authn.attributeRepository.ldap[0].attributes.mail=email
cas.authn.attributeRepository.ldap[0].attributes.uid=login
caowenbo commented 5 years ago

@piotrekfus91,I tested and found the problem you said. I have now added the analysis of Nested structures.At https://gerrit-review.googlesource.com/c/plugins/oauth/+/234973

davido commented 5 years ago

@caowenbo Thanks for tracking down and fixing the problem. Can someone please confirm that the latest patch set in https://gerrit-review.googlesource.com/c/plugins/oauth/+/234973 works as expected?

davido commented 2 years ago

The original change was abandoned, so closing this for now.