Merge pull request #3035 from mposolda/master

OIDC fixes
This commit is contained in:
Marek Posolda 2016-07-15 09:03:31 +02:00 committed by GitHub
commit a2ead8743f
13 changed files with 101 additions and 37 deletions

View file

@ -34,6 +34,7 @@ import org.keycloak.representations.AccessTokenResponse;
import org.keycloak.representations.IDToken;
import org.keycloak.common.util.KeycloakUriBuilder;
import org.keycloak.common.util.UriUtils;
import org.keycloak.util.TokenUtil;
import java.io.IOException;
import java.util.concurrent.atomic.AtomicLong;
@ -171,9 +172,9 @@ public class OAuthRequestAuthenticator {
if (idpHint != null && idpHint.length() > 0) {
redirectUriBuilder.queryParam(AdapterConstants.KC_IDP_HINT,idpHint);
}
if (scope != null && scope.length() > 0) {
redirectUriBuilder.queryParam(OAuth2Constants.SCOPE, scope);
}
scope = TokenUtil.attachOIDCScope(scope);
redirectUriBuilder.queryParam(OAuth2Constants.SCOPE, scope);
return redirectUriBuilder.build().toString();
}

View file

@ -20,6 +20,7 @@ package org.keycloak.jaxrs;
import org.keycloak.AbstractOAuthClient;
import org.keycloak.OAuth2Constants;
import org.keycloak.representations.AccessTokenResponse;
import org.keycloak.util.TokenUtil;
import javax.ws.rs.BadRequestException;
import javax.ws.rs.InternalServerErrorException;
@ -85,14 +86,13 @@ public class JaxrsOAuthClient extends AbstractOAuthClient {
}
public Response redirect(UriInfo uriInfo, String redirectUri) {
String state = getStateCode();
String scopeParam = TokenUtil.attachOIDCScope(scope);
UriBuilder uriBuilder = UriBuilder.fromUri(authUrl)
.queryParam(OAuth2Constants.CLIENT_ID, clientId)
.queryParam(OAuth2Constants.REDIRECT_URI, redirectUri)
.queryParam(OAuth2Constants.STATE, state);
if (scope != null) {
uriBuilder.queryParam(OAuth2Constants.SCOPE, scope);
}
.queryParam(OAuth2Constants.STATE, state)
.queryParam(OAuth2Constants.SCOPE, scopeParam);
URI url = uriBuilder.build();

View file

@ -103,8 +103,8 @@
initPromise.promise.success(function() {
kc.onReady && kc.onReady(kc.authenticated);
promise.setSuccess(kc.authenticated);
}).error(function() {
promise.setError();
}).error(function(errorData) {
promise.setError(errorData);
});
var configPromise = loadConfig(config);
@ -208,6 +208,8 @@
action = 'registrations';
}
var scope = (options && options.scope) ? "openid " + options.scope : "openid";
var url = getRealmUrl()
+ '/protocol/openid-connect/' + action
+ '?client_id=' + encodeURIComponent(kc.clientId)
@ -215,7 +217,8 @@
+ '&state=' + encodeURIComponent(state)
+ '&nonce=' + encodeURIComponent(nonce)
+ '&response_mode=' + encodeURIComponent(kc.responseMode)
+ '&response_type=' + encodeURIComponent(kc.responseType);
+ '&response_type=' + encodeURIComponent(kc.responseType)
+ '&scope=' + encodeURIComponent(scope);
if (options && options.prompt) {
url += '&prompt=' + encodeURIComponent(options.prompt);
@ -229,10 +232,6 @@
url += '&kc_idp_hint=' + encodeURIComponent(options.idpHint);
}
if (options && options.scope) {
url += '&scope=' + encodeURIComponent(options.scope);
}
if (options && options.locale) {
url += '&ui_locales=' + encodeURIComponent(options.locale);
}
@ -463,8 +462,9 @@
if (error) {
if (prompt != 'none') {
kc.onAuthError && kc.onAuthError();
promise && promise.setError();
var errorData = { error: error, error_description: oauth.error_description };
kc.onAuthError && kc.onAuthError(errorData);
promise && promise.setError(errorData);
} else {
promise && promise.setSuccess();
}
@ -1155,7 +1155,7 @@
}
var handleQueryParam = function(paramName, paramValue, oauth) {
var supportedOAuthParams = [ 'code', 'error', 'state' ];
var supportedOAuthParams = [ 'code', 'state', 'error', 'error_description' ];
for (var i = 0 ; i< supportedOAuthParams.length ; i++) {
if (paramName === supportedOAuthParams[i]) {

View file

@ -30,6 +30,7 @@ import org.keycloak.jose.jws.JWSInputException;
import org.keycloak.representations.AccessTokenResponse;
import org.keycloak.representations.IDToken;
import org.keycloak.common.util.KeycloakUriBuilder;
import org.keycloak.util.TokenUtil;
import javax.security.cert.X509Certificate;
import javax.servlet.http.Cookie;
@ -91,15 +92,15 @@ public class ServletOAuthClient extends KeycloakDeploymentDelegateOAuthClient {
String state = getStateCode();
KeycloakDeployment resolvedDeployment = resolveDeployment(getDeployment(), request);
String authUrl = resolvedDeployment.getAuthUrl().clone().build().toString();
String scopeParam = TokenUtil.attachOIDCScope(scope);
KeycloakUriBuilder uriBuilder = KeycloakUriBuilder.fromUri(authUrl)
.queryParam(OAuth2Constants.RESPONSE_TYPE, OAuth2Constants.CODE)
.queryParam(OAuth2Constants.CLIENT_ID, getClientId())
.queryParam(OAuth2Constants.REDIRECT_URI, redirectUri)
.queryParam(OAuth2Constants.STATE, state);
if (scope != null) {
uriBuilder.queryParam(OAuth2Constants.SCOPE, scope);
}
.queryParam(OAuth2Constants.STATE, state)
.queryParam(OAuth2Constants.SCOPE, scopeParam);
URI url = uriBuilder.build();
String stateCookiePath = this.stateCookiePath;

View file

@ -62,6 +62,15 @@ public interface OAuth2Constants {
// http://openid.net/specs/openid-connect-core-1_0.html#OfflineAccess
String OFFLINE_ACCESS = "offline_access";
// http://openid.net/specs/openid-connect-core-1_0.html#AuthRequest
String SCOPE_OPENID = "openid";
// http://openid.net/specs/openid-connect-core-1_0.html#ScopeClaims
String SCOPE_PROFILE = "profile";
String SCOPE_EMAIL = "email";
String SCOPE_ADDRESS = "address";
String SCOPE_PHONE = "phone";
String UI_LOCALES_PARAM = "ui_locales";

View file

@ -38,14 +38,30 @@ public class TokenUtil {
public static final String TOKEN_TYPE_OFFLINE = "Offline";
public static String attachOIDCScope(String scopeParam) {
if (scopeParam == null || scopeParam.isEmpty()) {
return OAuth2Constants.SCOPE_OPENID;
} else {
return OAuth2Constants.SCOPE_OPENID + " " + scopeParam;
}
}
public static boolean isOIDCRequest(String scopeParam) {
return hasScope(scopeParam, OAuth2Constants.SCOPE_OPENID);
}
public static boolean isOfflineTokenRequested(String scopeParam) {
if (scopeParam == null) {
return hasScope(scopeParam, OAuth2Constants.OFFLINE_ACCESS);
}
public static boolean hasScope(String scopeParam, String targetScope) {
if (scopeParam == null || targetScope == null) {
return false;
}
String[] scopes = scopeParam.split(" ");
for (String scope : scopes) {
if (OAuth2Constants.OFFLINE_ACCESS.equals(scope)) {
if (targetScope.equals(scope)) {
return true;
}
}

View file

@ -97,7 +97,11 @@ User <b id="subject"></b> made this request.
});
}
keycloak.init({ onLoad: 'login-required' }).success(reloadData);
keycloak.init({ onLoad: 'login-required' })
.success(reloadData)
.error(function(errorData) {
document.getElementById('customers').innerHTML = '<b>Failed to load data. Error: ' + JSON.stringify(errorData) + '</b>';
});
</script>

View file

@ -110,8 +110,8 @@
event('Auth Success');
};
keycloak.onAuthError = function () {
event('Auth Error');
keycloak.onAuthError = function (errorData) {
event("Auth Error: " + JSON.stringify(errorData) );
};
keycloak.onAuthRefreshSuccess = function () {

View file

@ -53,6 +53,7 @@ import org.keycloak.services.managers.ClientSessionCode;
import org.keycloak.services.messages.Messages;
import org.keycloak.services.resources.LoginActionsService;
import org.keycloak.services.util.CacheControlUtil;
import org.keycloak.util.TokenUtil;
/**
* @author <a href="mailto:sthorger@redhat.com">Stian Thorgersen</a>
@ -98,6 +99,7 @@ public class AuthorizationEndpoint extends AuthorizationEndpointBase {
KNOWN_REQ_PARAMS.add(OIDCLoginProtocol.PROMPT_PARAM);
KNOWN_REQ_PARAMS.add(AdapterConstants.KC_IDP_HINT);
KNOWN_REQ_PARAMS.add(OIDCLoginProtocol.NONCE_PARAM);
KNOWN_REQ_PARAMS.add(OIDCLoginProtocol.MAX_AGE_PARAM);
}
private enum Action {
@ -156,6 +158,10 @@ public class AuthorizationEndpoint extends AuthorizationEndpointBase {
return errorResponse;
}
if (!TokenUtil.isOIDCRequest(scope)) {
logger.oidcScopeMissing();
}
createClientSession();
// So back button doesn't work
CacheControlUtil.noBackButtonCacheControlHeader();
@ -259,6 +265,7 @@ public class AuthorizationEndpoint extends AuthorizationEndpointBase {
OIDCResponseMode defaultResponseMode = client.isImplicitFlowEnabled() ? OIDCResponseMode.FRAGMENT : OIDCResponseMode.QUERY;
if (responseType == null) {
logger.missingParameter(OAuth2Constants.RESPONSE_TYPE);
event.error(Errors.INVALID_REQUEST);
return redirectErrorToClient(defaultResponseMode, OAuthErrorException.INVALID_REQUEST, "Missing parameter: response_type");
}
@ -271,7 +278,7 @@ public class AuthorizationEndpoint extends AuthorizationEndpointBase {
action = Action.CODE;
}
} catch (IllegalArgumentException iae) {
logger.error(iae);
logger.error(iae.getMessage());
event.error(Errors.INVALID_REQUEST);
return redirectErrorToClient(defaultResponseMode, OAuthErrorException.UNSUPPORTED_RESPONSE_TYPE, null);
}
@ -280,6 +287,7 @@ public class AuthorizationEndpoint extends AuthorizationEndpointBase {
try {
parsedResponseMode = OIDCResponseMode.parse(responseMode, parsedResponseType);
} catch (IllegalArgumentException iae) {
logger.invalidParameter(OIDCLoginProtocol.RESPONSE_MODE_PARAM);
event.error(Errors.INVALID_REQUEST);
return redirectErrorToClient(defaultResponseMode, OAuthErrorException.INVALID_REQUEST, "Invalid parameter: response_mode");
}
@ -288,16 +296,19 @@ public class AuthorizationEndpoint extends AuthorizationEndpointBase {
// Disallowed by OIDC specs
if (parsedResponseType.isImplicitOrHybridFlow() && parsedResponseMode == OIDCResponseMode.QUERY) {
logger.responseModeQueryNotAllowed();
event.error(Errors.INVALID_REQUEST);
return redirectErrorToClient(defaultResponseMode, OAuthErrorException.INVALID_REQUEST, "Response_mode 'query' not allowed for implicit or hybrid flow");
}
if ((parsedResponseType.hasResponseType(OIDCResponseType.CODE) || parsedResponseType.hasResponseType(OIDCResponseType.NONE)) && !client.isStandardFlowEnabled()) {
logger.flowNotAllowed("Standard");
event.error(Errors.NOT_ALLOWED);
return redirectErrorToClient(parsedResponseMode, OAuthErrorException.UNSUPPORTED_RESPONSE_TYPE, "Client is not allowed to initiate browser login with given response_type. Standard flow is disabled for the client.");
}
if (parsedResponseType.isImplicitOrHybridFlow() && !client.isImplicitFlowEnabled()) {
logger.flowNotAllowed("Implicit");
event.error(Errors.NOT_ALLOWED);
return redirectErrorToClient(parsedResponseMode, OAuthErrorException.UNSUPPORTED_RESPONSE_TYPE, "Client is not allowed to initiate browser login with given response_type. Implicit flow is disabled for the client.");
}

View file

@ -32,6 +32,8 @@ import static org.jboss.logging.Logger.Level.ERROR;
import static org.jboss.logging.Logger.Level.FATAL;
import static org.jboss.logging.Logger.Level.INFO;
import static org.jboss.logging.Logger.Level.WARN;
import org.jboss.logging.annotations.Once;
import org.keycloak.email.EmailException;
import org.keycloak.events.EventListenerProvider;
import org.keycloak.models.ModelDuplicateException;
@ -402,4 +404,21 @@ public interface ServicesLogger extends BasicLogger {
@LogMessage(level = ERROR)
@Message(id=90, value="Failed to close ProviderSession")
void failedToCloseProviderSession(@Cause Throwable t);
@LogMessage(level = WARN)
@Message(id=91, value="Request is missing scope 'openid' so it's not treated as OIDC, but just pure OAuth2 request. This can have impact in future versions (eg. removed IDToken from the Token Response)")
@Once
void oidcScopeMissing();
@LogMessage(level = ERROR)
@Message(id=92, value="Missing parameter: %s")
void missingParameter(String paramName);
@LogMessage(level = ERROR)
@Message(id=93, value="Invalid parameter value for: %s")
void invalidParameter(String paramName);
@LogMessage(level = ERROR)
@Message(id=94, value="Client is not allowed to initiate browser login with given response_type. %s flow is disabled for the client.")
void flowNotAllowed(String flowName);
}

View file

@ -34,6 +34,7 @@ import org.keycloak.services.managers.Auth;
import org.keycloak.services.managers.AuthenticationManager;
import org.keycloak.services.util.CookieHelper;
import org.keycloak.common.util.UriUtils;
import org.keycloak.util.TokenUtil;
import javax.ws.rs.GET;
import javax.ws.rs.Path;
@ -237,15 +238,14 @@ public abstract class AbstractSecuredLocalService {
public Response redirect(UriInfo uriInfo, String redirectUri) {
String state = getStateCode();
String scopeParam = TokenUtil.attachOIDCScope(scope);
UriBuilder uriBuilder = UriBuilder.fromUri(authUrl)
.queryParam(OAuth2Constants.CLIENT_ID, clientId)
.queryParam(OAuth2Constants.REDIRECT_URI, redirectUri)
.queryParam(OAuth2Constants.STATE, state)
.queryParam(OAuth2Constants.RESPONSE_TYPE, OAuth2Constants.CODE);
if (scope != null) {
uriBuilder.queryParam(OAuth2Constants.SCOPE, scope);
}
.queryParam(OAuth2Constants.RESPONSE_TYPE, OAuth2Constants.CODE)
.queryParam(OAuth2Constants.SCOPE, scopeParam);
URI url = uriBuilder.build();

View file

@ -523,9 +523,10 @@ public class OAuthClient {
if(uiLocales != null){
b.queryParam(OAuth2Constants.UI_LOCALES_PARAM, uiLocales);
}
if (scope != null) {
b.queryParam(OAuth2Constants.SCOPE, scope);
}
String scopeParam = TokenUtil.attachOIDCScope(scope);
b.queryParam(OAuth2Constants.SCOPE, scopeParam);
if (maxAge != null) {
b.queryParam(OIDCLoginProtocol.MAX_AGE_PARAM, maxAge);
}

View file

@ -39,6 +39,7 @@ import org.keycloak.representations.AccessToken;
import org.keycloak.representations.RefreshToken;
import org.keycloak.util.BasicAuthHelper;
import org.keycloak.common.util.PemUtils;
import org.keycloak.util.TokenUtil;
import org.openqa.selenium.By;
import org.openqa.selenium.WebDriver;
@ -442,9 +443,10 @@ public class OAuthClient {
if(uiLocales != null){
b.queryParam(OAuth2Constants.UI_LOCALES_PARAM, uiLocales);
}
if (scope != null) {
b.queryParam(OAuth2Constants.SCOPE, scope);
}
String scopeParam = TokenUtil.attachOIDCScope(scope);
b.queryParam(OAuth2Constants.SCOPE, scopeParam);
return b.build(realm).toString();
}