diff --git a/forms/account-api/src/main/java/org/keycloak/account/AccountProvider.java b/forms/account-api/src/main/java/org/keycloak/account/AccountProvider.java old mode 100644 new mode 100755 index 281089a59b..ddc7b95ede --- a/forms/account-api/src/main/java/org/keycloak/account/AccountProvider.java +++ b/forms/account-api/src/main/java/org/keycloak/account/AccountProvider.java @@ -39,5 +39,7 @@ public interface AccountProvider extends Provider { AccountProvider setPasswordSet(boolean passwordSet); + AccountProvider setStateChecker(String stateChecker); + AccountProvider setFeatures(boolean social, boolean events, boolean passwordUpdateSupported); } diff --git a/forms/account-freemarker/src/main/java/org/keycloak/account/freemarker/FreeMarkerAccountProvider.java b/forms/account-freemarker/src/main/java/org/keycloak/account/freemarker/FreeMarkerAccountProvider.java index e7a632b820..7738b929b6 100755 --- a/forms/account-freemarker/src/main/java/org/keycloak/account/freemarker/FreeMarkerAccountProvider.java +++ b/forms/account-freemarker/src/main/java/org/keycloak/account/freemarker/FreeMarkerAccountProvider.java @@ -47,6 +47,7 @@ public class FreeMarkerAccountProvider implements AccountProvider { private RealmModel realm; private String[] referrer; private List events; + private String stateChecker; private List sessions; private boolean socialEnabled; private boolean eventsEnabled; @@ -107,6 +108,10 @@ public class FreeMarkerAccountProvider implements AccountProvider { } URI baseQueryUri = baseUriBuilder.build(); + if (stateChecker != null) { + attributes.put("stateChecker", stateChecker); + } + if (message != null) { attributes.put("message", new MessageBean(messages.containsKey(message) ? messages.getProperty(message) : message, messageType)); } @@ -115,7 +120,7 @@ public class FreeMarkerAccountProvider implements AccountProvider { attributes.put("referrer", new ReferrerBean(referrer)); } - attributes.put("url", new UrlBean(realm, theme, baseUri, baseQueryUri, uriInfo.getRequestUri())); + attributes.put("url", new UrlBean(realm, theme, baseUri, baseQueryUri, uriInfo.getRequestUri(), stateChecker)); attributes.put("features", new FeaturesBean(socialEnabled, eventsEnabled, passwordUpdateSupported)); @@ -127,7 +132,7 @@ public class FreeMarkerAccountProvider implements AccountProvider { attributes.put("totp", new TotpBean(user, baseUri)); break; case SOCIAL: - attributes.put("social", new AccountSocialBean(session, realm, user, uriInfo.getBaseUri())); + attributes.put("social", new AccountSocialBean(session, realm, user, uriInfo.getBaseUri(), stateChecker)); break; case LOG: attributes.put("log", new LogBean(events)); @@ -212,6 +217,12 @@ public class FreeMarkerAccountProvider implements AccountProvider { return this; } + @Override + public AccountProvider setStateChecker(String stateChecker) { + this.stateChecker = stateChecker; + return this; + } + @Override public AccountProvider setFeatures(boolean socialEnabled, boolean eventsEnabled, boolean passwordUpdateSupported) { this.socialEnabled = socialEnabled; diff --git a/forms/account-freemarker/src/main/java/org/keycloak/account/freemarker/model/AccountSocialBean.java b/forms/account-freemarker/src/main/java/org/keycloak/account/freemarker/model/AccountSocialBean.java index e84470f951..3a7650c549 100755 --- a/forms/account-freemarker/src/main/java/org/keycloak/account/freemarker/model/AccountSocialBean.java +++ b/forms/account-freemarker/src/main/java/org/keycloak/account/freemarker/model/AccountSocialBean.java @@ -25,7 +25,7 @@ public class AccountSocialBean { private final boolean removeLinkPossible; private final KeycloakSession session; - public AccountSocialBean(KeycloakSession session, RealmModel realm, UserModel user, URI baseUri) { + public AccountSocialBean(KeycloakSession session, RealmModel realm, UserModel user, URI baseUri, String stateChecker) { this.session = session; URI accountSocialUpdateUri = Urls.accountSocialUpdate(baseUri, realm.getName()); this.socialLinks = new LinkedList(); @@ -44,7 +44,11 @@ public class AccountSocialBean { availableLinks++; } String action = socialLink != null ? "remove" : "add"; - String actionUrl = UriBuilder.fromUri(accountSocialUpdateUri).queryParam("action", action).queryParam("provider_id", socialProviderId).build().toString(); + String actionUrl = UriBuilder.fromUri(accountSocialUpdateUri) + .queryParam("action", action) + .queryParam("provider_id", socialProviderId) + .queryParam("stateChecker", stateChecker) + .build().toString(); SocialLinkEntry entry = new SocialLinkEntry(socialLink, provider.getName(), actionUrl); this.socialLinks.add(entry); diff --git a/forms/account-freemarker/src/main/java/org/keycloak/account/freemarker/model/SessionsBean.java b/forms/account-freemarker/src/main/java/org/keycloak/account/freemarker/model/SessionsBean.java index de2f6e4d72..03ba5b3a0e 100755 --- a/forms/account-freemarker/src/main/java/org/keycloak/account/freemarker/model/SessionsBean.java +++ b/forms/account-freemarker/src/main/java/org/keycloak/account/freemarker/model/SessionsBean.java @@ -44,6 +44,8 @@ public class SessionsBean { this.session = session; } + public String getId() {return session.getId(); } + public String getIpAddress() { return session.getIpAddress(); } diff --git a/forms/account-freemarker/src/main/java/org/keycloak/account/freemarker/model/UrlBean.java b/forms/account-freemarker/src/main/java/org/keycloak/account/freemarker/model/UrlBean.java index 8a74b9bed6..5408a1d6de 100755 --- a/forms/account-freemarker/src/main/java/org/keycloak/account/freemarker/model/UrlBean.java +++ b/forms/account-freemarker/src/main/java/org/keycloak/account/freemarker/model/UrlBean.java @@ -16,13 +16,15 @@ public class UrlBean { private URI baseURI; private URI baseQueryURI; private URI currentURI; + private String stateChecker; - public UrlBean(RealmModel realm, Theme theme, URI baseURI, URI baseQueryURI, URI currentURI) { + public UrlBean(RealmModel realm, Theme theme, URI baseURI, URI baseQueryURI, URI currentURI, String stateChecker) { this.realm = realm.getName(); this.theme = theme; this.baseURI = baseURI; this.baseQueryURI = baseQueryURI; this.currentURI = currentURI; + this.stateChecker = stateChecker; } public String getAccessUrl() { @@ -54,11 +56,11 @@ public class UrlBean { } public String getSessionsLogoutUrl() { - return Urls.accountSessionsLogoutPage(baseQueryURI, realm).toString(); + return Urls.accountSessionsLogoutPage(baseQueryURI, realm, stateChecker).toString(); } public String getTotpRemoveUrl() { - return Urls.accountTotpRemove(baseQueryURI, realm).toString(); + return Urls.accountTotpRemove(baseQueryURI, realm, stateChecker).toString(); } public String getLogoutUrl() { diff --git a/forms/common-themes/src/main/resources/theme/account/base/account.ftl b/forms/common-themes/src/main/resources/theme/account/base/account.ftl index f83a62e287..f26f9206dc 100755 --- a/forms/common-themes/src/main/resources/theme/account/base/account.ftl +++ b/forms/common-themes/src/main/resources/theme/account/base/account.ftl @@ -12,6 +12,8 @@
+ +
diff --git a/forms/common-themes/src/main/resources/theme/account/base/password.ftl b/forms/common-themes/src/main/resources/theme/account/base/password.ftl index ccf7d0f9e8..af68d3b3ea 100755 --- a/forms/common-themes/src/main/resources/theme/account/base/password.ftl +++ b/forms/common-themes/src/main/resources/theme/account/base/password.ftl @@ -23,6 +23,8 @@
+ +
diff --git a/forms/common-themes/src/main/resources/theme/account/base/social.ftl b/forms/common-themes/src/main/resources/theme/account/base/social.ftl old mode 100644 new mode 100755 diff --git a/forms/common-themes/src/main/resources/theme/account/base/totp.ftl b/forms/common-themes/src/main/resources/theme/account/base/totp.ftl index 171a0b611e..f60bad1238 100755 --- a/forms/common-themes/src/main/resources/theme/account/base/totp.ftl +++ b/forms/common-themes/src/main/resources/theme/account/base/totp.ftl @@ -36,6 +36,7 @@
+
diff --git a/services/src/main/java/org/keycloak/services/managers/Auth.java b/services/src/main/java/org/keycloak/services/managers/Auth.java index 01bf896d80..7568c1b8bc 100755 --- a/services/src/main/java/org/keycloak/services/managers/Auth.java +++ b/services/src/main/java/org/keycloak/services/managers/Auth.java @@ -4,6 +4,7 @@ import org.keycloak.models.ApplicationModel; import org.keycloak.models.ClientModel; import org.keycloak.models.RealmModel; import org.keycloak.models.UserModel; +import org.keycloak.models.UserSessionModel; import org.keycloak.representations.AccessToken; /** @@ -16,17 +17,19 @@ public class Auth { private final AccessToken token; private final UserModel user; private final ClientModel client; + private final UserSessionModel session; - public Auth(RealmModel realm, AccessToken token, UserModel user, ClientModel client, boolean cookie) { + public Auth(RealmModel realm, AccessToken token, UserModel user, ClientModel client, UserSessionModel session, boolean cookie) { this.cookie = cookie; this.token = token; this.realm = realm; this.user = user; this.client = client; + this.session = session; } - public boolean isCookie() { + public boolean isCookieAuthenticated() { return cookie; } @@ -46,6 +49,10 @@ public class Auth { return token; } + public UserSessionModel getSession() { + return session; + } + public boolean hasRealmRole(String role) { if (cookie) { return user.hasRole(realm.getRole(role)); diff --git a/services/src/main/java/org/keycloak/services/resources/AccountService.java b/services/src/main/java/org/keycloak/services/resources/AccountService.java index 78b3961ff1..cfd3f746e6 100755 --- a/services/src/main/java/org/keycloak/services/resources/AccountService.java +++ b/services/src/main/java/org/keycloak/services/resources/AccountService.java @@ -145,16 +145,26 @@ public class AccountService { AuthenticationManager.AuthResult authResult = authManager.authenticateIdentityCookie(session, realm, uriInfo, clientConnection, headers); if (authResult != null) { - auth = new Auth(realm, authResult.getToken(), authResult.getUser(), application, true); + auth = new Auth(realm, authResult.getToken(), authResult.getUser(), application, authResult.getSession(), true); + } else { authResult = authManager.authenticateBearerToken(session, realm, uriInfo, clientConnection, headers); if (authResult != null) { - auth = new Auth(realm, authResult.getToken(), authResult.getUser(), application, false); + auth = new Auth(realm, authResult.getToken(), authResult.getUser(), application, authResult.getSession(), false); } } + // don't allow cors requests unless they were authenticated by an access token + // This is to prevent CSRF attacks. + if (auth != null && auth.isCookieAuthenticated()) { + if (headers.getRequestHeaders().containsKey("Origin")) { + throw new ForbiddenException(); + } + } + if (authResult != null) { UserSessionModel userSession = authResult.getSession(); if (userSession != null) { + account.setStateChecker(authResult.getSession().getId()); boolean associated = false; for (ClientSessionModel c : userSession.getClientSessions()) { if (c.getClient().equals(application)) { @@ -312,6 +322,34 @@ public class AccountService { return forwardToPage("sessions", AccountPages.SESSIONS); } + /** + * Check to see if form post has sessionId hidden field and match it against the session id. + * + * @param formData + */ + protected void csrfCheck(final MultivaluedMap formData) { + if (!auth.isCookieAuthenticated()) return; + if (auth.getSession() == null) return; + String stateChecker = formData.getFirst("stateChecker"); + if (!auth.getSession().getId().equals(stateChecker)) { + throw new ForbiddenException(); + } + + } + + /** + * Check to see if form post has sessionId hidden field and match it against the session id. + * + */ + protected void csrfCheck(String stateChecker) { + if (!auth.isCookieAuthenticated()) return; + if (auth.getSession() == null) return; + if (!auth.getSession().getId().equals(stateChecker)) { + throw new ForbiddenException(); + } + + } + /** * Update account information. * @@ -340,6 +378,8 @@ public class AccountService { return account.createResponse(AccountPages.ACCOUNT); } + csrfCheck(formData); + UserModel user = auth.getUser(); String error = Validation.validateUpdateProfileForm(formData); @@ -393,12 +433,13 @@ public class AccountService { @Path("sessions-logout") @GET - public Response processSessionsLogout() { + public Response processSessionsLogout(@QueryParam("stateChecker") String stateChecker) { if (auth == null) { return login("sessions"); } require(AccountRoles.MANAGE_ACCOUNT); + csrfCheck(stateChecker); UserModel user = auth.getUser(); session.sessions().removeUserSessions(realm, user); @@ -440,6 +481,8 @@ public class AccountService { return account.createResponse(AccountPages.TOTP); } + csrfCheck(formData); + UserModel user = auth.getUser(); String totp = formData.getFirst("totp"); @@ -494,6 +537,7 @@ public class AccountService { return account.createResponse(AccountPages.PASSWORD); } + csrfCheck(formData); UserModel user = auth.getUser(); boolean requireCurrent = isPasswordSet(user); @@ -546,12 +590,14 @@ public class AccountService { @Path("social-update") @GET public Response processSocialUpdate(@QueryParam("action") String action, - @QueryParam("provider_id") String providerId) { + @QueryParam("provider_id") String providerId, + @QueryParam("stateChecker") String stateChecker) { if (auth == null) { return login("social"); } require(AccountRoles.MANAGE_ACCOUNT); + csrfCheck(stateChecker); UserModel user = auth.getUser(); if (Validation.isEmpty(providerId)) { diff --git a/services/src/main/java/org/keycloak/services/resources/flows/Urls.java b/services/src/main/java/org/keycloak/services/resources/flows/Urls.java index 539674bff9..d766d6a937 100755 --- a/services/src/main/java/org/keycloak/services/resources/flows/Urls.java +++ b/services/src/main/java/org/keycloak/services/resources/flows/Urls.java @@ -68,8 +68,10 @@ public class Urls { return accountBase(baseUri).path(AccountService.class, "totpPage").build(realmId); } - public static URI accountTotpRemove(URI baseUri, String realmId) { - return accountBase(baseUri).path(AccountService.class, "processTotpRemove").build(realmId); + public static URI accountTotpRemove(URI baseUri, String realmId, String stateChecker) { + return accountBase(baseUri).path(AccountService.class, "processTotpRemove") + .queryParam("stateChecker", stateChecker) + .build(realmId); } public static URI accountLogPage(URI baseUri, String realmId) { @@ -80,8 +82,10 @@ public class Urls { return accountBase(baseUri).path(AccountService.class, "sessionsPage").build(realmId); } - public static URI accountSessionsLogoutPage(URI baseUri, String realmId) { - return accountBase(baseUri).path(AccountService.class, "processSessionsLogout").build(realmId); + public static URI accountSessionsLogoutPage(URI baseUri, String realmId, String stateChecker) { + return accountBase(baseUri).path(AccountService.class, "processSessionsLogout") + .queryParam("stateChecker", stateChecker) + .build(realmId); } public static URI accountLogout(URI baseUri, URI redirectUri, String realmId) {