Improve logging in case of OIDC Identity provider errors:
- log the full Redirection URL, when it contains an error parameter, or does not contain the state or code parameter - log the token endpoint URL (without - possibly confidential - params) and the response body, when the token endpoint does not return a success response Closes #23690
This commit is contained in:
parent
7c6f173d3a
commit
dd37e02140
4 changed files with 55 additions and 20 deletions
|
@ -248,6 +248,13 @@ public class SimpleHttp {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @return the URL without params
|
||||||
|
*/
|
||||||
|
public String getUrl() {
|
||||||
|
return url;
|
||||||
|
}
|
||||||
|
|
||||||
private Response makeRequest() throws IOException {
|
private Response makeRequest() throws IOException {
|
||||||
|
|
||||||
HttpRequestBase httpRequest = createHttpRequest();
|
HttpRequestBase httpRequest = createHttpRequest();
|
||||||
|
|
|
@ -479,7 +479,10 @@ public abstract class AbstractOAuth2IdentityProvider<C extends OAuth2IdentityPro
|
||||||
public Response authResponse(@QueryParam(AbstractOAuth2IdentityProvider.OAUTH2_PARAMETER_STATE) String state,
|
public Response authResponse(@QueryParam(AbstractOAuth2IdentityProvider.OAUTH2_PARAMETER_STATE) String state,
|
||||||
@QueryParam(AbstractOAuth2IdentityProvider.OAUTH2_PARAMETER_CODE) String authorizationCode,
|
@QueryParam(AbstractOAuth2IdentityProvider.OAUTH2_PARAMETER_CODE) String authorizationCode,
|
||||||
@QueryParam(OAuth2Constants.ERROR) String error) {
|
@QueryParam(OAuth2Constants.ERROR) String error) {
|
||||||
|
OAuth2IdentityProviderConfig providerConfig = provider.getConfig();
|
||||||
|
|
||||||
if (state == null) {
|
if (state == null) {
|
||||||
|
logErroneousRedirectUrlError("Redirection URL does not contain a state parameter", providerConfig);
|
||||||
return errorIdentityProviderLogin(Messages.IDENTITY_PROVIDER_MISSING_STATE_ERROR);
|
return errorIdentityProviderLogin(Messages.IDENTITY_PROVIDER_MISSING_STATE_ERROR);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -487,10 +490,8 @@ public abstract class AbstractOAuth2IdentityProvider<C extends OAuth2IdentityPro
|
||||||
AuthenticationSessionModel authSession = this.callback.getAndVerifyAuthenticationSession(state);
|
AuthenticationSessionModel authSession = this.callback.getAndVerifyAuthenticationSession(state);
|
||||||
session.getContext().setAuthenticationSession(authSession);
|
session.getContext().setAuthenticationSession(authSession);
|
||||||
|
|
||||||
OAuth2IdentityProviderConfig providerConfig = provider.getConfig();
|
|
||||||
|
|
||||||
if (error != null) {
|
if (error != null) {
|
||||||
logger.error(error + " for broker login " + providerConfig.getProviderId());
|
logErroneousRedirectUrlError("Redirection URL contains an error", providerConfig);
|
||||||
if (error.equals(ACCESS_DENIED)) {
|
if (error.equals(ACCESS_DENIED)) {
|
||||||
return callback.cancelled(providerConfig);
|
return callback.cancelled(providerConfig);
|
||||||
} else if (error.equals(OAuthErrorException.LOGIN_REQUIRED) || error.equals(OAuthErrorException.INTERACTION_REQUIRED)) {
|
} else if (error.equals(OAuthErrorException.LOGIN_REQUIRED) || error.equals(OAuthErrorException.INTERACTION_REQUIRED)) {
|
||||||
|
@ -500,23 +501,39 @@ public abstract class AbstractOAuth2IdentityProvider<C extends OAuth2IdentityPro
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
if (authorizationCode != null) {
|
if (authorizationCode == null) {
|
||||||
String response = generateTokenRequest(authorizationCode).asString();
|
logErroneousRedirectUrlError("Redirection URL neither contains a code nor error parameter",
|
||||||
|
providerConfig);
|
||||||
BrokeredIdentityContext federatedIdentity = provider.getFederatedIdentity(response);
|
return errorIdentityProviderLogin(Messages.IDENTITY_PROVIDER_MISSING_CODE_OR_ERROR_ERROR);
|
||||||
|
|
||||||
if (providerConfig.isStoreToken()) {
|
|
||||||
// make sure that token wasn't already set by getFederatedIdentity();
|
|
||||||
// want to be able to allow provider to set the token itself.
|
|
||||||
if (federatedIdentity.getToken() == null)federatedIdentity.setToken(response);
|
|
||||||
}
|
|
||||||
|
|
||||||
federatedIdentity.setIdpConfig(providerConfig);
|
|
||||||
federatedIdentity.setIdp(provider);
|
|
||||||
federatedIdentity.setAuthenticationSession(authSession);
|
|
||||||
|
|
||||||
return callback.authenticated(federatedIdentity);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
|
SimpleHttp simpleHttp = generateTokenRequest(authorizationCode);
|
||||||
|
String response;
|
||||||
|
try (SimpleHttp.Response simpleResponse = simpleHttp.asResponse()) {
|
||||||
|
int status = simpleResponse.getStatus();
|
||||||
|
boolean success = status >= 200 && status < 400;
|
||||||
|
response = simpleResponse.asString();
|
||||||
|
|
||||||
|
if (!success) {
|
||||||
|
logger.errorf("Unexpected response from token endpoint %s. status=%s, response=%s",
|
||||||
|
simpleHttp.getUrl(), status, response);
|
||||||
|
return errorIdentityProviderLogin(Messages.IDENTITY_PROVIDER_UNEXPECTED_ERROR);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
BrokeredIdentityContext federatedIdentity = provider.getFederatedIdentity(response);
|
||||||
|
|
||||||
|
if (providerConfig.isStoreToken()) {
|
||||||
|
// make sure that token wasn't already set by getFederatedIdentity();
|
||||||
|
// want to be able to allow provider to set the token itself.
|
||||||
|
if (federatedIdentity.getToken() == null)federatedIdentity.setToken(response);
|
||||||
|
}
|
||||||
|
|
||||||
|
federatedIdentity.setIdpConfig(providerConfig);
|
||||||
|
federatedIdentity.setIdp(provider);
|
||||||
|
federatedIdentity.setAuthenticationSession(authSession);
|
||||||
|
|
||||||
|
return callback.authenticated(federatedIdentity);
|
||||||
} catch (WebApplicationException e) {
|
} catch (WebApplicationException e) {
|
||||||
return e.getResponse();
|
return e.getResponse();
|
||||||
} catch (IdentityBrokerException e) {
|
} catch (IdentityBrokerException e) {
|
||||||
|
@ -524,10 +541,18 @@ public abstract class AbstractOAuth2IdentityProvider<C extends OAuth2IdentityPro
|
||||||
return errorIdentityProviderLogin(e.getMessageCode());
|
return errorIdentityProviderLogin(e.getMessageCode());
|
||||||
}
|
}
|
||||||
logger.error("Failed to make identity provider oauth callback", e);
|
logger.error("Failed to make identity provider oauth callback", e);
|
||||||
|
return errorIdentityProviderLogin(Messages.IDENTITY_PROVIDER_UNEXPECTED_ERROR);
|
||||||
} catch (Exception e) {
|
} catch (Exception e) {
|
||||||
logger.error("Failed to make identity provider oauth callback", e);
|
logger.error("Failed to make identity provider oauth callback", e);
|
||||||
|
return errorIdentityProviderLogin(Messages.IDENTITY_PROVIDER_UNEXPECTED_ERROR);
|
||||||
}
|
}
|
||||||
return errorIdentityProviderLogin(Messages.IDENTITY_PROVIDER_UNEXPECTED_ERROR);
|
}
|
||||||
|
|
||||||
|
private void logErroneousRedirectUrlError(String mainMessage, OAuth2IdentityProviderConfig providerConfig) {
|
||||||
|
String providerId = providerConfig.getProviderId();
|
||||||
|
String redirectionUrl = session.getContext().getUri().getRequestUri().toString();
|
||||||
|
|
||||||
|
logger.errorf("%s. providerId=%s, redirectionUrl=%s", mainMessage, providerId, redirectionUrl);
|
||||||
}
|
}
|
||||||
|
|
||||||
private Response errorIdentityProviderLogin(String message) {
|
private Response errorIdentityProviderLogin(String message) {
|
||||||
|
|
|
@ -185,6 +185,8 @@ public class Messages {
|
||||||
|
|
||||||
public static final String IDENTITY_PROVIDER_MISSING_STATE_ERROR = "identityProviderMissingStateMessage";
|
public static final String IDENTITY_PROVIDER_MISSING_STATE_ERROR = "identityProviderMissingStateMessage";
|
||||||
|
|
||||||
|
public static final String IDENTITY_PROVIDER_MISSING_CODE_OR_ERROR_ERROR = "identityProviderMissingCodeOrErrorMessage";
|
||||||
|
|
||||||
public static final String IDENTITY_PROVIDER_INVALID_RESPONSE = "identityProviderInvalidResponseMessage";
|
public static final String IDENTITY_PROVIDER_INVALID_RESPONSE = "identityProviderInvalidResponseMessage";
|
||||||
|
|
||||||
public static final String IDENTITY_PROVIDER_INVALID_SIGNATURE = "identityProviderInvalidSignatureMessage";
|
public static final String IDENTITY_PROVIDER_INVALID_SIGNATURE = "identityProviderInvalidSignatureMessage";
|
||||||
|
|
|
@ -343,6 +343,7 @@ cookieNotFoundMessage=Cookie not found. Please make sure cookies are enabled in
|
||||||
insufficientLevelOfAuthentication=The requested level of authentication has not been satisfied.
|
insufficientLevelOfAuthentication=The requested level of authentication has not been satisfied.
|
||||||
identityProviderUnexpectedErrorMessage=Unexpected error when authenticating with identity provider
|
identityProviderUnexpectedErrorMessage=Unexpected error when authenticating with identity provider
|
||||||
identityProviderMissingStateMessage=Missing state parameter in response from identity provider.
|
identityProviderMissingStateMessage=Missing state parameter in response from identity provider.
|
||||||
|
identityProviderMissingCodeOrErrorMessage=Missing code or error parameter in response from identity provider.
|
||||||
identityProviderInvalidResponseMessage=Invalid response from identity provider.
|
identityProviderInvalidResponseMessage=Invalid response from identity provider.
|
||||||
identityProviderInvalidSignatureMessage=Invalid signature in response from identity provider.
|
identityProviderInvalidSignatureMessage=Invalid signature in response from identity provider.
|
||||||
identityProviderNotFoundMessage=Could not find an identity provider with the identifier.
|
identityProviderNotFoundMessage=Could not find an identity provider with the identifier.
|
||||||
|
|
Loading…
Reference in a new issue