Proxy jwt bearer to support corporate trust #3309
Conversation
strehle
force-pushed
the
proxyJwtBearer
branch
5 times, most recently
from
33caea5 to
8276e37
Compare
strehle
force-pushed
the
proxyJwtBearer
branch
from
8276e37 to
f2541dd
Compare
strehle
changed the title
strehle
requested review from
a team,
tack-sap,
torsten-sap and
adrianhoelzl-sap
| try { | ||
| oidcMetadataFetcher.fetchMetadataAndUpdateDefinition(definition); | ||
| } catch (OidcMetadataFetchingException e) { | ||
| logger.warn("OidcMetadataFetchingException", e); | ||
| } | ||
| } | ||
|
|
||
| public IdentityProvider<OIDCIdentityProviderDefinition> getOidcProxyTokenExchange(HttpServletRequest request) { |
There was a problem hiding this comment.
Can we improve the name of this method? IMO, it indicates that we already perform a token exchange, even though that is not the case.
There was a problem hiding this comment.
for example: "getOidcProxyIdpForTokenExchange"
| if (clientAuth == null) { | ||
| throw new BadCredentialsException("No client authentication found."); | ||
| } | ||
| List<String> allowedProviders = null; |
There was a problem hiding this comment.
Please eliminate this variable by directly returning the result or null
| @@ -254,7 +256,17 @@ protected Authentication attemptTokenAuthentication(HttpServletRequest request, | |||
| log.debug(GRANT_TYPE_JWT_BEARER + " found. Attempting authentication with assertion"); | |||
| String assertion = request.getParameter("assertion"); | |||
| if (assertion != null && externalOAuthAuthenticationManager != null) { | |||
| log.debug("Attempting OIDC JWT authentication for token endpoint."); | |||
| IdentityProvider<OIDCIdentityProviderDefinition> oidcProxy = externalOAuthAuthenticationManager.getOidcProxyTokenExchange(request); | |||
| if (oidcProxy != null) { | |||
There was a problem hiding this comment.
Do I understand the general idea correctly?
We receive a JWT bearer request with a login_hint parameter. If the IdP described by that param has tokenExchangeEnabled, we forward the incoming JWT to the corporate IdP (also via a JWT bearer grant).
In contrast to that: if the flag is not enabled, we directly try to issue an access token for the user described by the incoming JWT.
There was a problem hiding this comment.
The "regular" JWT bearer flow against UAA can still be used if no login_hint parameter is passed in the request.
| @@ -32,6 +32,7 @@ public class OIDCIdentityProviderDefinition extends AbstractExternalOAuthIdentit | |||
| private URL discoveryUrl; | |||
| private boolean passwordGrantEnabled; | |||
| private boolean setForwardHeader; | |||
| private Boolean tokenExchangeEnabled; | |||
There was a problem hiding this comment.
Can you please add a Javadoc comment that describes the purpose of this flag?
| } else { | ||
| log.debug("Attempting OIDC JWT authentication for token endpoint."); | ||
| } | ||
|
|
||
| ExternalOAuthCodeToken token = new ExternalOAuthCodeToken(null, null, null, assertion, null, null); |
There was a problem hiding this comment.
Would it make sense to introduce a static factory method here? e.g. ExternalOAuthCodeToken.forAssertion(...)
| return retrieveTokenExchangeIdp(UaaLoginHint.parseRequestParameter(request.getParameter("login_hint")), getAllowedProviders()); | ||
| } | ||
|
|
||
| public List<String> getAllowedProviders() { |
There was a problem hiding this comment.
Can we make this method static?
Feature: Forward JWT bearer grant to trusted identity provider. Very similar to password grant.
Refactorings done, so that REST method to call trusted identity provider is re-used (from password grant) .
Reason: JWT bearer and Password grant need similar checks and parameters.
Why:
In a community CF usage the trust maybe is done origin by origin.
SAP uses the corporate origin trust model, so that access to CF is granted by a dedicated IdP which is controlled by SAP. Customers control their own corporate IdP and therefore the authentication requests need to be forward through the corporate Origin to the customer IdP. This model was created with #2505
See