Skip to content

Commit 64cb75b

Browse files
committed
[ELY-2868] When handling CachedIdentityAuthorizeCallback current authentication not prioritised.
1 parent a948e6b commit 64cb75b

File tree

4 files changed

+31
-20
lines changed

4 files changed

+31
-20
lines changed

auth/server/base/src/main/java/org/wildfly/security/auth/callback/CachedIdentityAuthorizeCallback.java

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ public class CachedIdentityAuthorizeCallback implements ExtendedCallback {
4646

4747
private final Function<SecurityDomain, IdentityCache> identityCache;
4848
private final boolean localCache;
49-
private Principal principal;
49+
private final Principal principal;
5050
private boolean authorized;
5151
private SecurityDomain securityDomain;
5252

@@ -85,11 +85,14 @@ public CachedIdentityAuthorizeCallback(Function<SecurityDomain, IdentityCache> i
8585
checkNotNullParam("identityCache", identityCache);
8686
this.identityCache = identityCache;
8787
this.localCache = localCache;
88+
this.principal = null;
8889
}
8990

9091
/**
9192
* Creates a new instance to authenticate, authorize and cache the identity associated with the given <code>name</code>.
9293
*
94+
* <p>By supplying a name authorizing the supplied name will be prioritised over restoring an identify from the cache</p>
95+
*
9396
* @param name the name associated with the identity
9497
* @param identityCache the identity cache
9598
*/
@@ -100,6 +103,8 @@ public CachedIdentityAuthorizeCallback(String name, IdentityCache identityCache)
100103
/**
101104
* Creates a new instance to authenticate, authorize and cache the identity associated with the given <code>principal</code>.
102105
*
106+
* <p>By supplying a {@code Principal} authorizing the supplied {@code Principal} will be prioritised over restoring an identify from the cache</p>
107+
*
103108
* @param principal the principal associated with the identity
104109
* @param identityCache the identity cache
105110
* @param localCache if true, indicates that authorization should be based on the given {@code identityCache} only. In case the mechanism
@@ -113,6 +118,8 @@ public CachedIdentityAuthorizeCallback(Principal principal, IdentityCache identi
113118
/**
114119
* Creates a new instance to authenticate, authorize and cache the identity associated with the given <code>principal</code>.
115120
*
121+
* <p>By supplying a {@code Principal} authorizing the supplied {@code Principal} will be prioritised over restoring an identify from the cache</p>
122+
*
116123
* @param principal the principal associated with the identity
117124
* @param identityCache the identity cache
118125
*/
@@ -125,6 +132,8 @@ public CachedIdentityAuthorizeCallback(Principal principal, IdentityCache identi
125132
*
126133
* <p>This constructor can be used to perform caching operations (e.g.: put, get and remove) in the context of a {@link SecurityDomain}.
127134
*
135+
* <p>By supplying a {@code Principal} authorizing the supplied {@code Principal} will be prioritised over restoring an identify from the cache</p>
136+
*
128137
* @param principal the principal associated with the identity
129138
* @param identityCache a function that creates an {@link IdentityCache} given a {@link SecurityDomain}
130139
* @param localCache if true, indicates that authorization should be based on the given {@code identityCache} only. In case the mechanism
@@ -156,7 +165,11 @@ public boolean isAuthorized() {
156165
public void setAuthorized(SecurityIdentity securityIdentity) {
157166
authorized = securityIdentity != null;
158167
if (authorized) {
159-
createDomainCache().put(securityIdentity);
168+
IdentityCache cache = createDomainCache();
169+
if (this.principal != null) {
170+
cache.remove();
171+
}
172+
cache.put(securityIdentity);
160173
} else {
161174
createDomainCache().remove();
162175
}
@@ -178,7 +191,7 @@ public Principal getPrincipal() {
178191
/**
179192
* Returns the authorization {@link Principal}.
180193
*
181-
* @return the principal (not {@code null})
194+
* @return the principal
182195
*/
183196
public Principal getAuthorizationPrincipal() {
184197
return this.principal;

auth/server/base/src/main/java/org/wildfly/security/auth/server/ServerAuthenticationContext.java

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1124,20 +1124,18 @@ private void handleOne(final Callback[] callbacks, final int idx) throws IOExcep
11241124
CachedIdentityAuthorizeCallback authorizeCallback = (CachedIdentityAuthorizeCallback) callback;
11251125
authorizeCallback.setSecurityDomain(stateRef.get().getSecurityDomain());
11261126
SecurityIdentity authorizedIdentity = null;
1127-
Principal principal = null;
1128-
SecurityIdentity identity = authorizeCallback.getIdentity();
1129-
if (identity != null && importIdentity(identity)) {
1127+
Principal principal = authorizeCallback.getAuthorizationPrincipal();
1128+
SecurityIdentity identity;
1129+
if (principal == null && (identity = authorizeCallback.getIdentity()) != null && importIdentity(identity)) {
11301130
authorizedIdentity = getAuthorizedIdentity();
1131-
} else {
1131+
} else if (principal == null) {
11321132
principal = authorizeCallback.getPrincipal();
1133-
if (principal == null) {
1134-
principal = authorizeCallback.getAuthorizationPrincipal();
1135-
}
1136-
if (principal != null) {
1137-
setAuthenticationPrincipal(principal);
1138-
if (authorize()) {
1139-
authorizedIdentity = getAuthorizedIdentity();
1140-
}
1133+
}
1134+
1135+
if (principal != null) {
1136+
setAuthenticationPrincipal(principal);
1137+
if (authorize()) {
1138+
authorizedIdentity = getAuthorizedIdentity();
11411139
}
11421140
}
11431141
log.tracef("Handling CachedIdentityAuthorizeCallback: principal = %s authorizedIdentity = %s", principal, authorizedIdentity);

http/sso/src/main/java/org/wildfly/security/http/util/sso/DefaultSingleSignOnSession.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ public DefaultSingleSignOnSession(SingleSignOnSessionContext context, HttpServer
7272
this.map.put(SINGLE_SIGN_ON_KEY, sso);
7373
this.request = checkNotNullParam("request", request);
7474
checkNotNullParam("sso", sso);
75-
this.ssoFactory = identity -> sso;
75+
this.ssoFactory = identity -> context.getSingleSignOnManager().create(sso.getMechanism(), sso.isProgrammatic(), identity);
7676
}
7777

7878
@Override
@@ -181,7 +181,7 @@ public void put(SecurityIdentity identity) {
181181

182182
@Override
183183
public CachedIdentity remove() {
184-
SingleSignOn sso = this.map.get(SINGLE_SIGN_ON_KEY);
184+
SingleSignOn sso = this.map.remove(SINGLE_SIGN_ON_KEY);
185185

186186
if (sso == null) return null;
187187

http/sso/src/main/java/org/wildfly/security/http/util/sso/SingleSignOnServerMechanismFactory.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -171,8 +171,7 @@ public void authenticationComplete(HttpServerMechanismsResponder responder) {
171171
String id = singleSignOnSession.getId();
172172
if (id != null) {
173173
HttpServerCookie cookie = getCookie(request);
174-
175-
if (cookie == null) {
174+
if (cookie == null || !id.equals(cookie.getValue())) {
176175
response.setResponseCookie(createCookie(id, -1));
177176
}
178177
}
@@ -194,7 +193,7 @@ public void authenticationComplete(HttpServerMechanismsResponder responder, Runn
194193
if (id != null) {
195194
HttpServerCookie cookie = getCookie(request);
196195

197-
if (cookie == null) {
196+
if (cookie == null || !id.equals(cookie.getValue())) {
198197
response.setResponseCookie(createCookie(id, -1));
199198
}
200199
}
@@ -273,6 +272,7 @@ private CallbackHandler createCallbackHandler(CallbackHandler callbackHandler, S
273272
}
274273
Principal principal = delegate.getAuthorizationPrincipal();
275274
if (principal != null) {
275+
276276
callbacks[i] = new CachedIdentityAuthorizeCallback(principal, singleSignOnSession) {
277277
@Override
278278
public void setAuthorized(SecurityIdentity securityIdentity) {

0 commit comments

Comments
 (0)