Skip to content

Commit

Permalink
KEYCLOAK-15270 Account REST API doesn't verify audience
Browse files Browse the repository at this point in the history
  • Loading branch information
@vmuzikar
vmuzikar authored and Bruno Oliveira da Silva committed Sep 14, 2020
1 parent b62d68a commit a9a719b
Show file tree
Hide file tree
Showing 13 changed files with 225 additions and 53 deletions.
2 changes: 1 addition & 1 deletion ...tension/src/main/java/org/keycloak/examples/domainextension/rest/ExampleRestResource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public class ExampleRestResource {

public ExampleRestResource(KeycloakSession session) {
this.session = session;
this.auth = new AppAuthManager().authenticateBearerToken(session, session.getContext().getRealm());
this.auth = new AppAuthManager.BearerTokenAuthenticator(session).authenticate();
}

@Path("companies")
Expand Down
10 changes: 4 additions & 6 deletions services/src/main/java/org/keycloak/authorization/util/Tokens.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,7 @@
public class Tokens {

public static AccessToken getAccessToken(KeycloakSession keycloakSession) {
AppAuthManager authManager = new AppAuthManager();
KeycloakContext context = keycloakSession.getContext();
AuthResult authResult = authManager.authenticateBearerToken(keycloakSession, context.getRealm(), context.getUri(), context.getConnection(), context.getRequestHeaders());
AuthResult authResult = new AppAuthManager.BearerTokenAuthenticator(keycloakSession).authenticate();

if (authResult != null) {
return authResult.getToken();
Expand All @@ -42,9 +40,9 @@ public static AccessToken getAccessToken(KeycloakSession keycloakSession) {
}

public static AccessToken getAccessToken(String accessToken, KeycloakSession keycloakSession) {
AppAuthManager authManager = new AppAuthManager();
KeycloakContext context = keycloakSession.getContext();
AuthResult authResult = authManager.authenticateBearerToken(accessToken, keycloakSession, context.getRealm(), context.getUri(), context.getConnection(), context.getRequestHeaders());
AuthResult authResult = new AppAuthManager.BearerTokenAuthenticator(keycloakSession)
.setTokenString(accessToken)
.authenticate();

if (authResult != null) {
return authResult.getToken();
Expand Down
3 changes: 1 addition & 2 deletions services/src/main/java/org/keycloak/protocol/oidc/endpoints/TokenEndpoint.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,6 @@
import org.keycloak.services.ServicesLogger;
import org.keycloak.services.Urls;
import org.keycloak.services.clientpolicy.ClientPolicyException;
import org.keycloak.services.clientpolicy.DefaultClientPolicyManager;
import org.keycloak.services.clientpolicy.TokenRefreshContext;
import org.keycloak.services.clientpolicy.TokenRequestContext;
import org.keycloak.services.managers.AppAuthManager;
Expand Down Expand Up @@ -797,7 +796,7 @@ public Response tokenExchange() {

}

AuthenticationManager.AuthResult authResult = AuthenticationManager.verifyIdentityToken(session, realm, session.getContext().getUri(), clientConnection, true, true, false, subjectToken, headers);
AuthenticationManager.AuthResult authResult = AuthenticationManager.verifyIdentityToken(session, realm, session.getContext().getUri(), clientConnection, true, true, null, false, subjectToken, headers);
if (authResult == null) {
event.detail(Details.REASON, "subject_token validation failure");
event.error(Errors.INVALID_TOKEN);
Expand Down
76 changes: 59 additions & 17 deletions services/src/main/java/org/keycloak/services/managers/AppAuthManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public AuthResult authenticateIdentityCookie(KeycloakSession session, RealmModel
*
* @return the token string or {@literal null}
*/
private String extractTokenStringFromAuthHeader(String authHeader) {
private static String extractTokenStringFromAuthHeader(String authHeader) {

if (authHeader == null) {
return null;
Expand Down Expand Up @@ -83,7 +83,7 @@ private String extractTokenStringFromAuthHeader(String authHeader) {
* @param headers
* @return the token string or {@literal null} if the Authorization header is not of type Bearer, or the token string is missing.
*/
public String extractAuthorizationHeaderTokenOrReturnNull(HttpHeaders headers) {
public static String extractAuthorizationHeaderTokenOrReturnNull(HttpHeaders headers) {
String authHeader = headers.getRequestHeaders().getFirst(HttpHeaders.AUTHORIZATION);
return extractTokenStringFromAuthHeader(authHeader);
}
Expand All @@ -95,7 +95,7 @@ public String extractAuthorizationHeaderTokenOrReturnNull(HttpHeaders headers) {
* @return the token string or {@literal null} of the Authorization header is missing
* @throws NotAuthorizedException if the Authorization header is not of type Bearer, or the token string is missing.
*/
public String extractAuthorizationHeaderToken(HttpHeaders headers) {
public static String extractAuthorizationHeaderToken(HttpHeaders headers) {
String authHeader = headers.getRequestHeaders().getFirst(HttpHeaders.AUTHORIZATION);
if (authHeader == null) {
return null;
Expand All @@ -107,23 +107,65 @@ public String extractAuthorizationHeaderToken(HttpHeaders headers) {
return tokenString;
}

public AuthResult authenticateBearerToken(KeycloakSession session, RealmModel realm) {
KeycloakContext ctx = session.getContext();
return authenticateBearerToken(session, realm, ctx.getUri(), ctx.getConnection(), ctx.getRequestHeaders());
}
public static class BearerTokenAuthenticator {
private KeycloakSession session;
private RealmModel realm;
private UriInfo uriInfo;
private ClientConnection connection;
private HttpHeaders headers;
private String tokenString;
private String audience;

public BearerTokenAuthenticator(KeycloakSession session) {
this.session = session;
}

public AuthResult authenticateBearerToken(KeycloakSession session) {
return authenticateBearerToken(session, session.getContext().getRealm(), session.getContext().getUri(), session.getContext().getConnection(), session.getContext().getRequestHeaders());
}
public BearerTokenAuthenticator setSession(KeycloakSession session) {
this.session = session;
return this;
}

public AuthResult authenticateBearerToken(KeycloakSession session, RealmModel realm, UriInfo uriInfo, ClientConnection connection, HttpHeaders headers) {
return authenticateBearerToken(extractAuthorizationHeaderToken(headers), session, realm, uriInfo, connection, headers);
}
public BearerTokenAuthenticator setRealm(RealmModel realm) {
this.realm = realm;
return this;
}

public AuthResult authenticateBearerToken(String tokenString, KeycloakSession session, RealmModel realm, UriInfo uriInfo, ClientConnection connection, HttpHeaders headers) {
if (tokenString == null) return null;
AuthResult authResult = verifyIdentityToken(session, realm, uriInfo, connection, true, true, false, tokenString, headers);
return authResult;
public BearerTokenAuthenticator setUriInfo(UriInfo uriInfo) {
this.uriInfo = uriInfo;
return this;
}

public BearerTokenAuthenticator setConnection(ClientConnection connection) {
this.connection = connection;
return this;
}

public BearerTokenAuthenticator setHeaders(HttpHeaders headers) {
this.headers = headers;
return this;
}

public BearerTokenAuthenticator setTokenString(String tokenString) {
this.tokenString = tokenString;
return this;
}

public BearerTokenAuthenticator setAudience(String audience) {
this.audience = audience;
return this;
}

public AuthResult authenticate() {
KeycloakContext ctx = session.getContext();
if (realm == null) realm = ctx.getRealm();
if (uriInfo == null) uriInfo = ctx.getUri();
if (connection == null) connection = ctx.getConnection();
if (headers == null) headers = ctx.getRequestHeaders();
if (tokenString == null) tokenString = extractAuthorizationHeaderToken(headers);
// audience can be null

return verifyIdentityToken(session, realm, uriInfo, connection, true, true, audience, false, tokenString, headers);
}
}

}
9 changes: 7 additions & 2 deletions services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -779,7 +779,7 @@ public static AuthResult authenticateIdentityCookie(KeycloakSession session, Rea
}

String tokenString = cookie.getValue();
AuthResult authResult = verifyIdentityToken(session, realm, session.getContext().getUri(), session.getContext().getConnection(), checkActive, false, true, tokenString, session.getContext().getRequestHeaders(), VALIDATE_IDENTITY_COOKIE);
AuthResult authResult = verifyIdentityToken(session, realm, session.getContext().getUri(), session.getContext().getConnection(), checkActive, false, null, true, tokenString, session.getContext().getRequestHeaders(), VALIDATE_IDENTITY_COOKIE);
if (authResult == null) {
expireIdentityCookie(realm, session.getContext().getUri(), session.getContext().getConnection());
expireOldIdentityCookie(realm, session.getContext().getUri(), session.getContext().getConnection());
Expand Down Expand Up @@ -1261,14 +1261,19 @@ public void ignore() {
}

public static AuthResult verifyIdentityToken(KeycloakSession session, RealmModel realm, UriInfo uriInfo, ClientConnection connection, boolean checkActive, boolean checkTokenType,
boolean isCookie, String tokenString, HttpHeaders headers, Predicate<? super AccessToken>... additionalChecks) {
String checkAudience, boolean isCookie, String tokenString, HttpHeaders headers, Predicate<? super AccessToken>... additionalChecks) {
try {
TokenVerifier<AccessToken> verifier = TokenVerifier.create(tokenString, AccessToken.class)
.withDefaultChecks()
.realmUrl(Urls.realmIssuer(uriInfo.getBaseUri(), realm.getName()))
.checkActive(checkActive)
.checkTokenType(checkTokenType)
.withChecks(additionalChecks);

if (checkAudience != null) {
verifier.audience(checkAudience);
}

String kid = verifier.getHeader().getKeyId();
String algorithm = verifier.getHeader().getAlgorithm().name();

Expand Down
7 changes: 5 additions & 2 deletions services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -448,8 +448,11 @@ private Response getToken(String providerId, boolean forceRetrieval) {
this.event.event(EventType.IDENTITY_PROVIDER_RETRIEVE_TOKEN);

try {
AppAuthManager authManager = new AppAuthManager();
AuthenticationManager.AuthResult authResult = authManager.authenticateBearerToken(this.session, this.realmModel, this.session.getContext().getUri(), this.clientConnection, this.request.getHttpHeaders());
AuthenticationManager.AuthResult authResult = new AppAuthManager.BearerTokenAuthenticator(session)
.setRealm(realmModel)
.setConnection(clientConnection)
.setHeaders(request.getHttpHeaders())
.authenticate();

if (authResult != null) {
AccessToken token = authResult.getToken();
Expand Down
5 changes: 4 additions & 1 deletion services/src/main/java/org/keycloak/services/resources/account/AccountLoader.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,10 @@ private boolean isDeprecatedFormsAccountConsole(Theme theme) {
}

private AccountRestService getAccountRestService(ClientModel client, String versionStr) {
AuthenticationManager.AuthResult authResult = new AppAuthManager().authenticateBearerToken(session);
AuthenticationManager.AuthResult authResult = new AppAuthManager.BearerTokenAuthenticator(session)
.setAudience(client.getClientId())
.authenticate();

if (authResult == null) {
throw new NotAuthorizedException("Bearer token required");
}
Expand Down
9 changes: 6 additions & 3 deletions services/src/main/java/org/keycloak/services/resources/admin/AdminConsole.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,12 +87,10 @@ public class AdminConsole {
@Context
protected Providers providers;

protected AppAuthManager authManager;
protected RealmModel realm;

public AdminConsole(RealmModel realm) {
this.realm = realm;
this.authManager = new AppAuthManager();
}

public static class WhoAmI {
Expand Down Expand Up @@ -195,7 +193,12 @@ public ClientManager.InstallationAdapterConfig config() {
@NoCache
public Response whoAmI(final @Context HttpHeaders headers) {
RealmManager realmManager = new RealmManager(session);
AuthenticationManager.AuthResult authResult = authManager.authenticateBearerToken(session, realm, session.getContext().getUri(), clientConnection, headers);
AuthenticationManager.AuthResult authResult = new AppAuthManager.BearerTokenAuthenticator(session)
.setRealm(realm)
.setConnection(clientConnection)
.setHeaders(headers)
.authenticate();

if (authResult == null) {
return Response.status(401).build();
}
Expand Down
12 changes: 8 additions & 4 deletions services/src/main/java/org/keycloak/services/resources/admin/AdminRoot.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,15 +72,13 @@ public class AdminRoot {
@Context
protected HttpResponse response;

protected AppAuthManager authManager;
protected TokenManager tokenManager;

@Context
protected KeycloakSession session;

public AdminRoot() {
this.tokenManager = new TokenManager();
this.authManager = new AppAuthManager();
}

public static UriBuilder adminBaseUrl(UriInfo uriInfo) {
Expand Down Expand Up @@ -153,7 +151,7 @@ public AdminConsole getAdminConsole(final @PathParam("realm") String name) {


protected AdminAuth authenticateRealmAdminRequest(HttpHeaders headers) {
String tokenString = authManager.extractAuthorizationHeaderToken(headers);
String tokenString = AppAuthManager.extractAuthorizationHeaderToken(headers);
if (tokenString == null) throw new NotAuthorizedException("Bearer");
AccessToken token;
try {
Expand All @@ -169,7 +167,13 @@ protected AdminAuth authenticateRealmAdminRequest(HttpHeaders headers) {
throw new NotAuthorizedException("Unknown realm in token");
}
session.getContext().setRealm(realm);
AuthenticationManager.AuthResult authResult = authManager.authenticateBearerToken(session, realm, session.getContext().getUri(), clientConnection, headers);

AuthenticationManager.AuthResult authResult = new AppAuthManager.BearerTokenAuthenticator(session)
.setRealm(realm)
.setConnection(clientConnection)
.setHeaders(headers)
.authenticate();

if (authResult == null) {
logger.debug("Token not valid");
throw new NotAuthorizedException("Bearer");
Expand Down
2 changes: 1 addition & 1 deletion ...viders/src/main/java/org/keycloak/testsuite/domainextension/rest/ExampleRestResource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public class ExampleRestResource {

public ExampleRestResource(KeycloakSession session) {
this.session = session;
this.auth = new AppAuthManager().authenticateBearerToken(session, session.getContext().getRealm());
this.auth = new AppAuthManager.BearerTokenAuthenticator(session).authenticate();
}

@Path("companies")
Expand Down
8 changes: 4 additions & 4 deletions ...llian/tests/base/src/test/java/org/keycloak/testsuite/account/AccountFormServiceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1203,15 +1203,15 @@ public void applicationsVisibilityNoScopesNoConsent() throws Exception {

Map<String, AccountApplicationsPage.AppEntry> apps = applicationsPage.getApplications();
Assert.assertThat(apps.keySet(), containsInAnyOrder(
/* "root-url-client", */ "Account", "Account Console", "test-app", "test-app-scope", "third-party", "test-app-authz", "My Named Test App", "Test App Named - ${client_account}", "direct-grant"));
/* "root-url-client", */ "Account", "Account Console", "test-app", "test-app-scope", "third-party", "test-app-authz", "My Named Test App", "Test App Named - ${client_account}", "direct-grant", "custom-audience"));

rsu.add(testRealm().roles().get("user").toRepresentation())
.update();

driver.navigate().refresh();
apps = applicationsPage.getApplications();
Assert.assertThat(apps.keySet(), containsInAnyOrder(
"root-url-client", "Account", "Account Console", "test-app", "test-app-scope", "third-party", "test-app-authz", "My Named Test App", "Test App Named - ${client_account}", "direct-grant"));
"root-url-client", "Account", "Account Console", "test-app", "test-app-scope", "third-party", "test-app-authz", "My Named Test App", "Test App Named - ${client_account}", "direct-grant", "custom-audience"));
}
}

Expand All @@ -1230,7 +1230,7 @@ public void applicationsVisibilityNoScopesAndConsent() throws Exception {

Map<String, AccountApplicationsPage.AppEntry> apps = applicationsPage.getApplications();
Assert.assertThat(apps.keySet(), containsInAnyOrder(
"root-url-client", "Account", "Account Console", "test-app", "test-app-scope", "third-party", "test-app-authz", "My Named Test App", "Test App Named - ${client_account}", "direct-grant"));
"root-url-client", "Account", "Account Console", "test-app", "test-app-scope", "third-party", "test-app-authz", "My Named Test App", "Test App Named - ${client_account}", "direct-grant", "custom-audience"));
}
}

Expand All @@ -1245,7 +1245,7 @@ public void applications() {
applicationsPage.assertCurrent();

Map<String, AccountApplicationsPage.AppEntry> apps = applicationsPage.getApplications();
Assert.assertThat(apps.keySet(), containsInAnyOrder("root-url-client", "Account", "Account Console", "Broker", "test-app", "test-app-scope", "third-party", "test-app-authz", "My Named Test App", "Test App Named - ${client_account}", "direct-grant"));
Assert.assertThat(apps.keySet(), containsInAnyOrder("root-url-client", "Account", "Account Console", "Broker", "test-app", "test-app-scope", "third-party", "test-app-authz", "My Named Test App", "Test App Named - ${client_account}", "direct-grant", "custom-audience"));

AccountApplicationsPage.AppEntry accountEntry = apps.get("Account");
Assert.assertThat(accountEntry.getRolesAvailable(), containsInAnyOrder(
Expand Down
Loading

0 comments on commit a9a719b

Please sign in to comment.