support getting caller and group information from multiple tokens #26719
Conversation
|
#build |
|
Your personal build request is at https://wasrtc.hursley.ibm.com:9443/jazz/resource/itemOid/com.ibm.team.build.BuildResult/_-fsC4HLdEe6LyICjST0JEw Target locations of links might be accessible only to IBM employees. |
arunavemulapalli
force-pushed
the
caller-from-multiple-tokens-updates
branch
from
88c6bc8 to
f876e18
Compare
|
The build arunavemulapalli-26719-20231024-2038 For help analyzing your personal build, go to https://libh-proxy1.fyre.ibm.com/cognitive/buildAnalysis.html?uuid=_-fsC4HLdEe6LyICjST0JEw |
|
The build arunavemulapalli-26719-2-20231025-1441 For help analyzing your personal build, go to https://libh-proxy1.fyre.ibm.com/cognitive/buildAnalysis.html?uuid=_tPX1kHN1Ee6LyICjST0JEw |
|
#libby |
arunavemulapalli
force-pushed
the
caller-from-multiple-tokens-updates
branch
from
1189bff to
bf4e530
Compare
|
The build arunavemulapalli-26719-2-20231028-1555 For help analyzing your personal build, go to https://libh-proxy1.fyre.ibm.com/cognitive/buildAnalysis.html?uuid=_Vwua0HXbEe6p7N33NVgeoQ |
|
The build arunavemulapalli-26719-2-20231031-2120 For help analyzing your personal build, go to https://libh-proxy1.fyre.ibm.com/cognitive/buildAnalysis.html?uuid=_RQ_MMHhkEe6p7N33NVgeoQ |
arunavemulapalli
force-pushed
the
caller-from-multiple-tokens-updates
branch
from
a48f897 to
6f189a7
Compare
|
#libby |
|
The build arunavemulapalli-26719-2-20231106-1602 For help analyzing your personal build, go to https://libh-proxy1.fyre.ibm.com/cognitive/buildAnalysis.html?uuid=_XffuwHz3Ee6pmuZkFgD-jA |
arunavemulapalli
force-pushed
the
caller-from-multiple-tokens-updates
branch
from
6f189a7 to
e1607c0
Compare
|
#libby |
arunavemulapalli
force-pushed
the
caller-from-multiple-tokens-updates
branch
from
e1607c0 to
a9709b9
Compare
… token, access token, user info address review comments social - OidcLoginConfigImpl update Fix the groupId / aud retrieval issue configuration update configuration update and add implementation to OidcLoginConfigImpl Update to ignore the exception from finding userinfo and continue with the other 2 tokens Update to use space instead of "," to fix the configuration issue update to take default value when the new config attribute is not used
|
The build arunavemulapalli-26719-2-20231107-2016 |
|
L2 message review not required. |
|
#libby |
| @@ -562,7 +567,14 @@ private void processConfigProps(Map<String, Object> props) { | |||
| // checkValidationEndpointUrl(); | |||
|
|
|||
| // validateAuthzTokenEndpoints(); //TODO: update tests to expect the error if the validation here fails | |||
|
|
|||
| String tokens = configUtils.getConfigAttributeWithDefaultValue(props, CFG_KEY_TOKEN_ORDER_TOFETCH_CALLER_CLAIMS, "IDToken");//getStringArrayConfigAttribute(props, CFG_KEY_TOKEN_ORDER_TOFETCH_CALLER_CLAIMS); | |||
| /* if (tokens == null) { | |||
| @@ -298,3 +298,8 @@ pkceCodeChallengeMethod.S256=S256 | |||
|
|
|||
| tokenRequestOriginHeader=Token request origin header | |||
| tokenRequestOriginHeader.desc=Specifies the value to use in the Origin HTTP header that is included in the HTTP POST request to the token endpoint of the OpenID Connect provider. If not specified, an Origin HTTP header is not included in the request. | |||
|
|
|||
| tokenOrderToFetchCallerClaims=Specify one or more tokens to fetch caller claims | |||
| tokenOrderToFetchCallerClaims.desc=Specifies the order of the token/s to fetch the caller name and group claim values. If the claim does not exist in the first token, then the OpenID Connect client will continue the search with other tokens in the list | |||
There was a problem hiding this comment.
Remove /
change "will continue" to "continues"
| tokenOrderToFetchCallerClaims=Specify one or more tokens to fetch caller claims | ||
| tokenOrderToFetchCallerClaims.desc=Specifies the order of the token/s to fetch the caller name and group claim values. If the claim does not exist in the first token, then the OpenID Connect client will continue the search with other tokens in the list | ||
| tokenOrderToFetchCallerClaims.IDToken=Use IDToken only to determine the caller claims | ||
| tokenOrderToFetchCallerClaims.ACCESSAndIDAndUITokens=Use AccessToken, IDToken and Userinfo tokens in this order, to determine the caller claims. |
There was a problem hiding this comment.
Uses the AccessToken, IDToken, and UserInfo tokens, in that order, to determine the caller claims.
|
|
||
| tokenOrderToFetchCallerClaims=Specify one or more tokens to fetch caller claims | ||
| tokenOrderToFetchCallerClaims.desc=Specifies the order of the token/s to fetch the caller name and group claim values. If the claim does not exist in the first token, then the OpenID Connect client will continue the search with other tokens in the list | ||
| tokenOrderToFetchCallerClaims.IDToken=Use IDToken only to determine the caller claims |
There was a problem hiding this comment.
Uses only the IDToken to determine the caller claims
|
|
||
| public static final String USER_PRINCIPAL_NAME = "upn"; | ||
|
|
||
| public static final String GROUP = "groupIds"; |
There was a problem hiding this comment.
Would it make more sense to call this GROUP_IDS instead of GROUP?
| @@ -40,6 +40,7 @@ public class TokenEndpointServlet extends HttpServlet { | |||
| private static final long serialVersionUID = 1L; | |||
| private final String servletName = "TokenEndpointServlet"; | |||
| private String token = null; | |||
| private String idt = null; | |||
There was a problem hiding this comment.
Would really prefer this to be called something like overrideIdToken or idTokenOverride or idTokenRequestParameter so it's more descriptive than the cryptic idt.
| trustAliasName="rs256" | ||
| signatureAlgorithm="RS256" | ||
| tokenOrderToFetchCallerClaims="AccessToken IDToken Userinfo" | ||
| mapIdentityToRegistryUser="yes" |
There was a problem hiding this comment.
mapIdentityToRegistryUser takes a boolean as a value, so the value needs to either be "true" or "false".
| com.ibm.ws.webcontainer.security.*=all=enabled:\ | ||
| com.ibm.oauth.*=all=enabled:\ | ||
| com.ibm.wsspi.security.oauth20.*=all=enabled:\ | ||
| org.apache.http.client.*=all |
There was a problem hiding this comment.
Should also add io.openliberty.security*=all.
| com.ibm.ws.webcontainer.security.*=all=enabled:\ | ||
| com.ibm.oauth.*=all=enabled:\ | ||
| com.ibm.wsspi.security.oauth20.*=all=enabled:\ | ||
| org.apache.http.client.*=all |
There was a problem hiding this comment.
Should add io.openliberty.security*=all.
| userInfoClaims = getClaimsFromUserInfo(userInfoStr); | ||
|
|
||
| } catch (Exception e) { | ||
| Tr.error(tc, "Invalid user info: " + userInfoStr); |
There was a problem hiding this comment.
| Tr.error(tc, "Invalid user info: " + userInfoStr); | |
| Tr.debug(tc, "Invalid user info: " + userInfoStr); |
The Tr.error() method emits messages to the console.log and messages.log that require translation. The Tr.debug() method should be used here instead.
There was a problem hiding this comment.
This is a higher priority issue to fix.
| oidcResult = new ProviderAuthenticationResult(AuthResult.SUCCESS, HttpServletResponse.SC_OK, null, null, props, null); | ||
| if (isRunningBetaMode()) { | ||
| createWASOidcSession(oidcClientRequest, jwtClaims, clientConfig); | ||
| //createWASOidcSession(oidcClientRequest, jwtClaims, clientConfig); |
| uid = clientConfig.getUserIdentityToCreateSubject(); | ||
| } | ||
|
|
||
| String[] userClaimIdentifiers = {attrUsedToCreateSubject, uid}; |
There was a problem hiding this comment.
This isn't an ideal way to return data back from the method. It's not clear to callers that [0] happens to be the name of the attribute name and [1] happens to be the value.
| @SuppressWarnings("unchecked") | ||
| List<String> getGroupIds(ConvergedClientConfig clientConfig, List<String> tokensOrderToFetchCallerClaims, Map<String, JwtClaims> tokenClaimsMap) throws MalformedClaimException { |
There was a problem hiding this comment.
| @SuppressWarnings("unchecked") | |
| List<String> getGroupIds(ConvergedClientConfig clientConfig, List<String> tokensOrderToFetchCallerClaims, Map<String, JwtClaims> tokenClaimsMap) throws MalformedClaimException { | |
| @SuppressWarnings("unchecked") | |
| @FFDCIgnore(MalformedClaimException.class) | |
| List<String> getGroupIds(ConvergedClientConfig clientConfig, List<String> tokensOrderToFetchCallerClaims, Map<String, JwtClaims> tokenClaimsMap) throws MalformedClaimException { |
| List<String> groupIds = null; | ||
| try { | ||
| groupIds = getClaimValueFromTokens(groupIdsClaim, List.class, tokensOrderToFetchCallerClaims, tokenClaimsMap); | ||
| } catch (Exception e) { |
There was a problem hiding this comment.
| } catch (Exception e) { | |
| } catch (MalformedClaimException e) { |
|
thanks @ayoho , @arkarkala for the review. I will get the review issues done |
arunavemulapalli
force-pushed
the
caller-from-multiple-tokens-updates
branch
from
a9709b9 to
ee5dcdd
Compare
|
#build |
|
Your personal build request is at https://wasrtc.hursley.ibm.com:9443/jazz/resource/itemOid/com.ibm.team.build.BuildResult/_yOrhMH_3Ee6pmuZkFgD-jA Target locations of links might be accessible only to IBM employees. |
|
#libby |
Code analysis and actionsDO NOT DELETE THIS COMMENT.
|
|
The build arunavemulapalli-26719-20231110-1143 For help analyzing your personal build, go to https://libh-proxy1.fyre.ibm.com/cognitive/buildAnalysis.html?uuid=_yOrhMH_3Ee6pmuZkFgD-jA |
| @@ -298,3 +298,8 @@ pkceCodeChallengeMethod.S256=S256 | |||
|
|
|||
| tokenRequestOriginHeader=Token request origin header | |||
| tokenRequestOriginHeader.desc=Specifies the value to use in the Origin HTTP header that is included in the HTTP POST request to the token endpoint of the OpenID Connect provider. If not specified, an Origin HTTP header is not included in the request. | |||
|
|
|||
| tokenOrderToFetchCallerClaims=Specify one or more tokens to fetch caller claims | |||
update oidc client config, runtime
For #25460
Fixes #26888