New OIDC PIP #24
New OIDC PIP #24
Conversation
| this.decoder = decoder; | ||
| } | ||
|
|
||
| protected boolean isOidcProfile(Request request) { |
There was a problem hiding this comment.
rename to isOidcProfileRequest
|
|
||
| if (!isOidcProfile(request)) { | ||
| String msg = "Request doesn't match OIDC profile"; | ||
| LOG.error(msg); |
There was a problem hiding this comment.
Is this really an error condition? If the PEP supports multiple profiles it shouldn't be reported as an error.
| TokenInfo tokenInfo = decoder.decodeAccessToken(accessToken.get()); | ||
|
|
||
| if (!tokenInfo.getIntrospection().isActive()) { | ||
| String msg = String.format("Invalid access token: '%s'", request); |
There was a problem hiding this comment.
Is this really an error condition? With the X.509 PIP we simply remove the X.509 attributes when a credential is no longer valid if I am not mistaken.
| throw new PIPProcessingException(msg); | ||
| } | ||
|
|
||
| tokenService.cleanOidcAttributes(request); |
There was a problem hiding this comment.
This should probably be moved up, as the first instruction after having determined that the OIDC profile applies.
| import org.glite.authz.common.model.Request; | ||
| import org.glite.authz.oidc.client.model.TokenInfo; | ||
|
|
||
| public interface OidcProfileToken { |
There was a problem hiding this comment.
This can be renamed OidcProfileTokenService
| import org.slf4j.Logger; | ||
| import org.slf4j.LoggerFactory; | ||
|
|
||
| public class OidcProfileTokenImpl implements OidcProfileToken { |
There was a problem hiding this comment.
Rename to OidcProfileTokenServiceImpl
| for (Attribute attr : sub.getAttributes()) { | ||
| if (ID_ATTRIBUTE_OIDC_ACCESS_TOKEN.equals(attr.getId())) { | ||
| Set<Object> values = attr.getValues(); | ||
| accessToken = Optional.of(values.iterator().next().toString()); |
There was a problem hiding this comment.
If values is empty a NPE will be raised here. Add a check that controls that the attribute has at least 1 value or otherwise raises an exception (with a meaningful error message)
| attributesToAdd.add(oidcSubject); | ||
|
|
||
| if (tokenInfo.getIntrospection() == null) { | ||
| LOG.warn("No introspection data into access token."); |
There was a problem hiding this comment.
Error message should be
LOG.warn("No introspection data returned by token service for access token : %s", token);token is the token string
| } | ||
|
|
||
| if (tokenInfo.getUserinfo() == null) { | ||
| LOG.warn("No user info data into access token."); |
There was a problem hiding this comment.
Error message should be
LOG.warn("No userinfo data returned by token service for access token : %s", token);token is the token string
| LOG.debug("Sending request to OIDC client '{}'", oidcHttpService.getOidcClientUrl()); | ||
|
|
||
| String response = oidcHttpService.postRequest(accessToken); | ||
| LOG.debug("Get response from OIDC client: '{}'", response); |
There was a problem hiding this comment.
Get response -> Response from OIDC client:
First implementation of OIDC PIP