Skip to content
This repository was archived by the owner on Sep 28, 2022. It is now read-only.

Commit 39bc00a

Browse files
author
Dominik Frantisek Bucik
committed
fix: 🐛 Fix ACR for implicit and authorization_code flows
BREAKING CHANGE: 🧨 Database needs to be updated: `ALTER TABLE saved_user_auth DROP source_class; ALTER TABLE saved_user_auth ADD COLUMN acr VARCHAR(1024);`
1 parent b4cd6a4 commit 39bc00a

File tree

11 files changed

+93
-51
lines changed

11 files changed

+93
-51
lines changed

perun-oidc-server-webapp/src/main/resources/db/hsql/hsql_database_tables.sql

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ CREATE TABLE IF NOT EXISTS authentication_holder_request_parameter (
8686

8787
CREATE TABLE IF NOT EXISTS saved_user_auth (
8888
id BIGINT GENERATED BY DEFAULT AS IDENTITY(START WITH 1) PRIMARY KEY,
89+
acr VARCHAR(1024),
8990
name VARCHAR(1024),
9091
authenticated BOOLEAN,
9192
source_class VARCHAR(2048)

perun-oidc-server-webapp/src/main/resources/db/mysql/mysql_database_tables.sql

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ CREATE TABLE IF NOT EXISTS authentication_holder_request_parameter (
8585

8686
CREATE TABLE IF NOT EXISTS saved_user_auth (
8787
id BIGINT AUTO_INCREMENT PRIMARY KEY,
88+
acr VARCHAR(1024),
8889
name VARCHAR(1024),
8990
authenticated BOOLEAN,
9091
source_class VARCHAR(2048)

perun-oidc-server-webapp/src/main/resources/db/psql/psql_database_tables.sql

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ CREATE TABLE IF NOT EXISTS authentication_holder_request_parameter (
8686

8787
CREATE TABLE IF NOT EXISTS saved_user_auth (
8888
id BIGSERIAL PRIMARY KEY,
89+
acr VARCHAR(1024),
8990
name VARCHAR(1024),
9091
authenticated BOOLEAN,
9192
source_class VARCHAR(2048)

perun-oidc-server/src/main/java/cz/muni/ics/oauth2/model/SavedUserAuthentication.java

Lines changed: 44 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,10 @@
1717
package cz.muni.ics.oauth2.model;
1818

1919
import cz.muni.ics.oauth2.model.convert.SimpleGrantedAuthorityStringConverter;
20+
import cz.muni.ics.oidc.saml.SamlPrincipal;
2021
import java.util.Collection;
2122
import java.util.HashSet;
23+
import java.util.stream.Collectors;
2224
import javax.persistence.Basic;
2325
import javax.persistence.CollectionTable;
2426
import javax.persistence.Column;
@@ -32,8 +34,14 @@
3234
import javax.persistence.JoinColumn;
3335
import javax.persistence.Table;
3436
import javax.persistence.Transient;
37+
import lombok.ToString;
38+
import lombok.extern.slf4j.Slf4j;
39+
import org.opensaml.saml2.core.AuthnContext;
40+
import org.opensaml.saml2.core.AuthnContextClassRef;
41+
import org.opensaml.saml2.core.AuthnStatement;
3542
import org.springframework.security.core.Authentication;
3643
import org.springframework.security.core.GrantedAuthority;
44+
import org.springframework.security.providers.ExpiringUsernameAuthenticationToken;
3745

3846
/**
3947
* This class stands in for an original Authentication object.
@@ -42,6 +50,8 @@
4250
*/
4351
@Entity
4452
@Table(name="saved_user_auth")
53+
@Slf4j
54+
@ToString
4555
public class SavedUserAuthentication implements Authentication {
4656

4757
private static final long serialVersionUID = -1804249963940323488L;
@@ -50,18 +60,21 @@ public class SavedUserAuthentication implements Authentication {
5060
private String name;
5161
private Collection<GrantedAuthority> authorities;
5262
private boolean authenticated;
53-
private String sourceClass;
63+
private String acr;
5464

5565
public SavedUserAuthentication(Authentication src) {
5666
setName(src.getName());
5767
setAuthorities(new HashSet<>(src.getAuthorities()));
5868
setAuthenticated(src.isAuthenticated());
59-
60-
if (src instanceof SavedUserAuthentication) {
61-
// if we're copying in a saved auth, carry over the original class name
62-
setSourceClass(((SavedUserAuthentication) src).getSourceClass());
63-
} else {
64-
setSourceClass(src.getClass().getName());
69+
if (src instanceof ExpiringUsernameAuthenticationToken) {
70+
ExpiringUsernameAuthenticationToken token = (ExpiringUsernameAuthenticationToken) src;
71+
this.acr = ((SamlPrincipal) token.getPrincipal()).getSamlCredential()
72+
.getAuthenticationAssertion()
73+
.getAuthnStatements().stream()
74+
.map(AuthnStatement::getAuthnContext)
75+
.map(AuthnContext::getAuthnContextClassRef)
76+
.map(AuthnContextClassRef::getAuthnContextClassRef)
77+
.collect(Collectors.joining());
6578
}
6679
}
6780

@@ -85,6 +98,10 @@ public String getName() {
8598
return name;
8699
}
87100

101+
public void setName(String name) {
102+
this.name = name;
103+
}
104+
88105
@Override
89106
@ElementCollection(fetch = FetchType.EAGER)
90107
@CollectionTable(name="saved_user_auth_authority", joinColumns=@JoinColumn(name="owner_id"))
@@ -94,22 +111,18 @@ public Collection<GrantedAuthority> getAuthorities() {
94111
return authorities;
95112
}
96113

97-
@Override
98-
@Transient
99-
public Object getCredentials() {
100-
return "";
114+
public void setAuthorities(Collection<GrantedAuthority> authorities) {
115+
this.authorities = authorities;
101116
}
102117

103-
@Override
104-
@Transient
105-
public Object getDetails() {
106-
return null;
118+
@Basic
119+
@Column(name = "acr")
120+
public String getAcr() {
121+
return acr;
107122
}
108123

109-
@Override
110-
@Transient
111-
public Object getPrincipal() {
112-
return getName();
124+
public void setAcr(String acr) {
125+
this.acr = acr;
113126
}
114127

115128
@Override
@@ -124,22 +137,22 @@ public void setAuthenticated(boolean isAuthenticated) throws IllegalArgumentExce
124137
this.authenticated = isAuthenticated;
125138
}
126139

127-
@Basic
128-
@Column(name="source_class")
129-
public String getSourceClass() {
130-
return sourceClass;
131-
}
132-
133-
public void setSourceClass(String sourceClass) {
134-
this.sourceClass = sourceClass;
140+
@Override
141+
@Transient
142+
public Object getCredentials() {
143+
return "";
135144
}
136145

137-
public void setName(String name) {
138-
this.name = name;
146+
@Override
147+
@Transient
148+
public Object getDetails() {
149+
return null;
139150
}
140151

141-
public void setAuthorities(Collection<GrantedAuthority> authorities) {
142-
this.authorities = authorities;
152+
@Override
153+
@Transient
154+
public Object getPrincipal() {
155+
return getName();
143156
}
144157

145158
}

perun-oidc-server/src/main/java/cz/muni/ics/oauth2/token/DeviceTokenGranter.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import cz.muni.ics.oauth2.service.DeviceCodeService;
2323
import cz.muni.ics.oauth2.web.DeviceEndpoint;
2424
import java.util.Date;
25+
import lombok.extern.slf4j.Slf4j;
2526
import org.springframework.beans.factory.annotation.Autowired;
2627
import org.springframework.security.oauth2.common.exceptions.InvalidGrantException;
2728
import org.springframework.security.oauth2.provider.ClientDetails;
@@ -42,6 +43,7 @@
4243
*
4344
*/
4445
@Component("deviceTokenGranter")
46+
@Slf4j
4547
public class DeviceTokenGranter extends AbstractTokenGranter {
4648

4749
public static final String GRANT_TYPE = "urn:ietf:params:oauth:grant-type:device_code";

perun-oidc-server/src/main/java/cz/muni/ics/oauth2/web/DeviceEndpoint.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -243,8 +243,9 @@ public String readUserCode(@RequestParam("user_code") String userCode, ModelMap
243243

244244
@PreAuthorize("hasRole('ROLE_USER')")
245245
@RequestMapping(value = "/" + USER_URL + "/approve", method = RequestMethod.POST)
246-
public String approveDevice(@RequestParam("user_code") String userCode, @RequestParam(value = "user_oauth_approval") Boolean approve, ModelMap model, Authentication auth, HttpSession session) {
247-
246+
public String approveDevice(@RequestParam("user_code") String userCode,
247+
@RequestParam(value = "user_oauth_approval") Boolean approve,
248+
ModelMap model, Authentication auth, HttpSession session) {
248249
AuthorizationRequest authorizationRequest = (AuthorizationRequest) session.getAttribute("authorizationRequest");
249250
DeviceCode dc = (DeviceCode) session.getAttribute("deviceCode");
250251

perun-oidc-server/src/main/java/cz/muni/ics/oidc/saml/PerunSamlAuthenticationProvider.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,7 @@ public PerunSamlAuthenticationProvider(List<String> adminIds) {
3030
@Override
3131
protected Object getPrincipal(SAMLCredential credential, Object userDetail) {
3232
PerunUser user = (PerunUser) userDetail;
33-
return new User(String.valueOf(user.getId()), credential.getRemoteEntityID(),
34-
getEntitlements(credential, userDetail));
33+
return new SamlPrincipal(user.getId(), credential, getEntitlements(credential, userDetail));
3534
}
3635

3736
@Override
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
package cz.muni.ics.oidc.saml;
2+
3+
import java.util.Collection;
4+
import lombok.Getter;
5+
import lombok.Setter;
6+
import lombok.ToString;
7+
import org.springframework.security.core.GrantedAuthority;
8+
import org.springframework.security.core.userdetails.User;
9+
import org.springframework.security.saml.SAMLCredential;
10+
11+
@Getter
12+
@Setter
13+
@ToString
14+
public class SamlPrincipal extends User {
15+
16+
private Long perunUserId;
17+
private SAMLCredential samlCredential;
18+
19+
public SamlPrincipal(Long perunUserId,
20+
SAMLCredential samlCredential,
21+
Collection<? extends GrantedAuthority> authorities) {
22+
super(String.valueOf(perunUserId), "[PROTECTED]", authorities);
23+
this.perunUserId = perunUserId;
24+
this.samlCredential = samlCredential;
25+
}
26+
27+
}

perun-oidc-server/src/main/java/cz/muni/ics/oidc/server/PerunOIDCTokenService.java

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,6 @@
1818
import net.minidev.json.JSONArray;
1919
import org.springframework.beans.factory.annotation.Autowired;
2020
import org.springframework.security.oauth2.provider.OAuth2Request;
21-
import org.springframework.web.context.request.RequestAttributes;
22-
import org.springframework.web.context.request.RequestContextHolder;
23-
import org.springframework.web.context.request.ServletRequestAttributes;
2421

2522
/**
2623
* Modifies ID Token.
@@ -49,8 +46,11 @@ public PerunOIDCTokenService(UserInfoService userInfoService,
4946
}
5047

5148
@Override
52-
protected void addCustomIdTokenClaims(JWTClaimsSet.Builder idClaims, ClientDetailsEntity client, OAuth2Request request,
53-
String sub, OAuth2AccessTokenEntity accessToken)
49+
protected void addCustomIdTokenClaims(JWTClaimsSet.Builder idClaims,
50+
ClientDetailsEntity client,
51+
OAuth2Request request,
52+
String sub,
53+
OAuth2AccessTokenEntity accessToken)
5454
{
5555
log.debug("modifying ID token");
5656
String userId = accessToken.getAuthenticationHolder().getAuthentication().getName();
@@ -73,18 +73,17 @@ protected void addCustomIdTokenClaims(JWTClaimsSet.Builder idClaims, ClientDetai
7373
}
7474
}
7575

76-
String acr = getAuthnContextClass();
77-
if (acr != null) {
78-
log.debug("adding to ID token claim acr with value {}", acr);
79-
idClaims.claim("acr", acr);
76+
if (accessToken.getAuthenticationHolder() != null
77+
&& accessToken.getAuthenticationHolder().getUserAuth() != null)
78+
{
79+
String acr = accessToken.getAuthenticationHolder().getUserAuth().getAcr();
80+
if (acr != null) {
81+
log.debug("adding to ID token claim acr with value {}", acr);
82+
idClaims.claim("acr", acr);
83+
}
8084
}
8185
}
8286

83-
private String getAuthnContextClass() {
84-
ServletRequestAttributes attr = (ServletRequestAttributes) RequestContextHolder.currentRequestAttributes();
85-
return (String) attr.getAttribute(SESSION_PARAM_ACR, RequestAttributes.SCOPE_SESSION);
86-
}
87-
8887
/**
8988
* Converts claim values from com.google.gson.JsonElement to net.minidev.json.JSONObject or primitive value
9089
*

perun-oidc-server/src/main/java/cz/muni/ics/openid/connect/service/impl/DefaultOIDCTokenService.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,6 @@
6666
* @author Amanda Anganes
6767
*
6868
*/
69-
@Service
7069
@Slf4j
7170
public class DefaultOIDCTokenService implements OIDCTokenService {
7271

0 commit comments

Comments
 (0)