Skip to content

Commit

Permalink
Combine all scope filtering logic into one ScopeFilter
Browse files Browse the repository at this point in the history
  • Loading branch information
federicaagostini authored and enricovianello committed Jan 28, 2025
1 parent 1e9c200 commit 103aba3
Show file tree
Hide file tree
Showing 27 changed files with 1,096 additions and 776 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,15 @@
import it.infn.mw.iam.authn.util.Authorities;
import it.infn.mw.iam.core.ExtendedAuthenticationToken;
import it.infn.mw.iam.persistence.model.IamAccount;
import it.infn.mw.iam.persistence.model.IamAuthority;
import it.infn.mw.iam.persistence.repository.IamAccountRepository;

@SuppressWarnings("deprecation")
@Component
public class AccountUtils {

private static final IamAuthority ROLE_ADMIN = new IamAuthority("ROLE_ADMIN");

IamAccountRepository accountRepo;

public AccountUtils(IamAccountRepository accountRepo) {
Expand All @@ -53,6 +57,14 @@ public boolean isAdmin(Authentication auth) {
return auth.getAuthorities().contains(Authorities.ROLE_ADMIN);
}

public boolean isAdmin(IamAccount account) {
if (account == null || account.getAuthorities().isEmpty()) {
return false;
}

return account.getAuthorities().contains(ROLE_ADMIN);
}

public boolean isPreAuthenticated(Authentication auth) {
if (auth == null || auth.getAuthorities().isEmpty()) {
return false;
Expand All @@ -73,6 +85,7 @@ public boolean isAuthenticated(Authentication auth) {
}

public Optional<IamAccount> getAuthenticatedUserAccount(Authentication authn) {

if (!isAuthenticated(authn)) {
return Optional.empty();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -293,8 +293,6 @@ FilterRegistrationBean<EnforceAupFilter> aupSignatureCheckFilter(AUPSignatureChe
return frb;
}



@Bean
ScopeMatcherRegistry customScopeMatchersRegistry(ScopeMatchersProperties properties,
SystemScopeRepository scopeRepo) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@
import it.infn.mw.iam.core.oauth.scope.IamSystemScopeService;
import it.infn.mw.iam.core.oauth.scope.matchers.ScopeMatcherOAuthRequestValidator;
import it.infn.mw.iam.core.oauth.scope.matchers.ScopeMatcherRegistry;
import it.infn.mw.iam.core.oauth.scope.pdp.IamScopeFilter;
import it.infn.mw.iam.core.oauth.scope.pdp.ScopeFilter;
import it.infn.mw.iam.core.oidc.IamClientValidationService;
import it.infn.mw.iam.core.userinfo.IamUserInfoInterceptor;

Expand Down Expand Up @@ -162,8 +162,7 @@ OAuth2RequestValidator requestValidator(ScopeMatcherRegistry registry) {
}

@Bean
OAuth2RequestFactory requestFactory(IamScopeFilter scopeFilter,
JWTProfileResolver profileResolver) {
OAuth2RequestFactory requestFactory(ScopeFilter scopeFilter, JWTProfileResolver profileResolver) {
return new IamOAuth2RequestFactory(clientDetailsEntityService(), scopeFilter, profileResolver);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,11 @@
import it.infn.mw.iam.audit.events.tokens.AccessTokenIssuedEvent;
import it.infn.mw.iam.audit.events.tokens.RefreshTokenIssuedEvent;
import it.infn.mw.iam.config.IamProperties;
import it.infn.mw.iam.core.oauth.scope.pdp.ScopeFilter;
import it.infn.mw.iam.persistence.repository.IamOAuthAccessTokenRepository;
import it.infn.mw.iam.persistence.repository.IamOAuthRefreshTokenRepository;

@SuppressWarnings("deprecation")
@Service("defaultOAuth2ProviderTokenService")
@Primary
public class IamTokenService extends DefaultOAuth2ProviderTokenService {
Expand All @@ -51,16 +53,18 @@ public class IamTokenService extends DefaultOAuth2ProviderTokenService {
private final IamOAuthRefreshTokenRepository refreshTokenRepo;
private final ApplicationEventPublisher eventPublisher;
private final IamProperties iamProperties;
private final ScopeFilter scopeFilter;


public IamTokenService(IamOAuthAccessTokenRepository atRepo,
IamOAuthRefreshTokenRepository rtRepo, ApplicationEventPublisher publisher,
IamProperties iamProperties) {
IamProperties iamProperties, ScopeFilter scopeFilter) {

this.accessTokenRepo = atRepo;
this.refreshTokenRepo = rtRepo;
this.eventPublisher = publisher;
this.iamProperties = iamProperties;
this.scopeFilter = scopeFilter;
}

@Override
Expand Down Expand Up @@ -90,10 +94,9 @@ public void revokeRefreshToken(OAuth2RefreshTokenEntity refreshToken) {
}

@Override
@SuppressWarnings("deprecation")
public OAuth2AccessTokenEntity createAccessToken(OAuth2Authentication authentication) {

OAuth2AccessTokenEntity token = super.createAccessToken(authentication);
OAuth2AccessTokenEntity token = super.createAccessToken(scopeFilter.filterScopes(authentication));

if (iamProperties.getClient().isTrackLastUsed()) {
updateClientLastUsed(token);
Expand All @@ -107,14 +110,13 @@ public OAuth2AccessTokenEntity createAccessToken(OAuth2Authentication authentica
public OAuth2RefreshTokenEntity createRefreshToken(ClientDetailsEntity client,
AuthenticationHolderEntity authHolder) {

OAuth2RefreshTokenEntity token = super.createRefreshToken(client, authHolder);
OAuth2RefreshTokenEntity token = super.createRefreshToken(client, scopeFilter.filterScopes(authHolder));

eventPublisher.publishEvent(new RefreshTokenIssuedEvent(this, token));
return token;
}

@Override
@SuppressWarnings("deprecation")
public OAuth2AccessTokenEntity refreshAccessToken(String refreshTokenValue,
TokenRequest authRequest) {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
import com.google.common.base.Joiner;

import it.infn.mw.iam.core.oauth.profile.JWTProfileResolver;
import it.infn.mw.iam.core.oauth.scope.pdp.IamScopeFilter;
import it.infn.mw.iam.core.oauth.scope.pdp.ScopeFilter;

@SuppressWarnings("deprecation")
public class IamOAuth2RequestFactory extends ConnectOAuth2RequestFactory {
Expand All @@ -50,33 +50,31 @@ public class IamOAuth2RequestFactory extends ConnectOAuth2RequestFactory {
public static final String AUD = "aud";
public static final String PASSWORD_GRANT = "password";

private final IamScopeFilter scopeFilter;
private final ScopeFilter scopeFilter;

private final JWTProfileResolver profileResolver;

private final Joiner joiner = Joiner.on(' ');
private final ClientDetailsEntityService clientDetailsService;

public IamOAuth2RequestFactory(ClientDetailsEntityService clientDetailsService,
IamScopeFilter scopeFilter, JWTProfileResolver profileResolver) {
ScopeFilter scopeFilter, JWTProfileResolver profileResolver) {
super(clientDetailsService);
this.clientDetailsService = clientDetailsService;
this.scopeFilter = scopeFilter;
this.profileResolver = profileResolver;
}


@Override
public AuthorizationRequest createAuthorizationRequest(Map<String, String> inputParams) {

Authentication authn = SecurityContextHolder.getContext().getAuthentication();

if (authn != null && !(authn instanceof AnonymousAuthenticationToken)) {
final Set<String> requestedScopes =
Set<String> requestedScopes =
OAuth2Utils.parseParameterList(inputParams.get(OAuth2Utils.SCOPE));

scopeFilter.filterScopes(requestedScopes, authn);
inputParams.put(OAuth2Utils.SCOPE, joiner.join(requestedScopes));
inputParams.put(OAuth2Utils.SCOPE, joiner.join(scopeFilter.filterScopes(requestedScopes, authn)));
}

AuthorizationRequest authzRequest = super.createAuthorizationRequest(inputParams);
Expand Down Expand Up @@ -138,6 +136,8 @@ public OAuth2Request createOAuth2Request(ClientDetails client, TokenRequest toke
public TokenRequest createTokenRequest(Map<String, String> requestParameters,
ClientDetails authenticatedClient) {

Authentication authn = SecurityContextHolder.getContext().getAuthentication();

String clientId = requestParameters.get(OAuth2Utils.CLIENT_ID);
if (clientId == null) {
clientId = authenticatedClient.getClientId();
Expand All @@ -161,7 +161,6 @@ public TokenRequest createTokenRequest(Map<String, String> requestParameters,
}
}

return new TokenRequest(requestParameters, clientId, scopes, grantType);
return new TokenRequest(requestParameters, clientId, scopeFilter.filterScopes(scopes, authn), grantType);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -25,20 +25,19 @@
import java.net.URISyntaxException;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;

import org.apache.http.client.utils.URIBuilder;
import org.mitre.oauth2.model.ClientDetailsEntity;
import org.mitre.oauth2.service.ClientDetailsEntityService;
import org.mitre.oauth2.service.SystemScopeService;
import org.mitre.openid.connect.view.HttpCodeView;
import org.mitre.openid.connect.view.JsonErrorView;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.http.HttpStatus;
import org.springframework.security.access.prepost.PreAuthorize;
import org.springframework.security.core.Authentication;
import org.springframework.security.oauth2.common.exceptions.OAuth2Exception;
import org.springframework.security.oauth2.common.util.OAuth2Utils;
import org.springframework.security.oauth2.provider.AuthorizationRequest;
import org.springframework.security.oauth2.provider.endpoint.RedirectResolver;
Expand All @@ -51,27 +50,32 @@
import com.google.common.base.Splitter;
import com.google.common.base.Strings;

import it.infn.mw.iam.persistence.repository.client.IamClientRepository;

@SuppressWarnings("deprecation")
@Controller
@SessionAttributes("authorizationRequest")
public class IamOAuthConfirmationController {

@Autowired
private ClientDetailsEntityService clientService;
private static final Logger logger =
LoggerFactory.getLogger(IamOAuthConfirmationController.class);

private IamClientRepository clientRepository;

@Autowired
private SystemScopeService scopeService;

@Autowired
private RedirectResolver redirectResolver;

@Autowired
private IamUserApprovalUtils userApprovalUtils;

private static final Logger logger =
LoggerFactory.getLogger(IamOAuthConfirmationController.class);
public IamOAuthConfirmationController(IamClientRepository clientRepository, SystemScopeService scopeService,
RedirectResolver redirectResolver, IamUserApprovalUtils userApprovalUtils) {

this.clientRepository = clientRepository;
this.scopeService = scopeService;
this.redirectResolver = redirectResolver;
this.userApprovalUtils = userApprovalUtils;
}

@PreAuthorize("hasRole('ROLE_USER')")
@GetMapping(AUTHZ_CODE_URL)
Expand All @@ -81,30 +85,23 @@ public String confimAccess(Map<String, Object> model,

String prompt = (String) authRequest.getExtensions().get(PROMPT);
List<String> prompts = Splitter.on(PROMPT_SEPARATOR).splitToList(Strings.nullToEmpty(prompt));
ClientDetailsEntity client = null;

try {
client = clientService.loadClientByClientId(authRequest.getClientId());
} catch (OAuth2Exception e) {
logger.error("confirmAccess: OAuth2Exception was thrown when attempting to load client", e);
model.put(HttpCodeView.CODE, HttpStatus.BAD_REQUEST);
return HttpCodeView.VIEWNAME;
} catch (IllegalArgumentException e) {
logger.error(
"confirmAccess: IllegalArgumentException was thrown when attempting to load client", e);
String clientId = authRequest.getClientId();
if (clientId == null || clientId.isBlank()) {
model.put(HttpCodeView.CODE, HttpStatus.BAD_REQUEST);
model.put(JsonErrorView.ERROR, "invalid_client");
model.put(JsonErrorView.ERROR_MESSAGE, "Null client id");
return HttpCodeView.VIEWNAME;
}

if (client == null) {
logger.error("confirmAccess: could not find client {}", authRequest.getClientId());
Optional<ClientDetailsEntity> client = clientRepository.findByClientId(clientId);
if (client.isEmpty()) {
model.put(HttpCodeView.CODE, HttpStatus.NOT_FOUND);
return HttpCodeView.VIEWNAME;
}

if (prompts.contains("none")) {

String url = redirectResolver.resolveRedirect(authRequest.getRedirectUri(), client);
String url = redirectResolver.resolveRedirect(authRequest.getRedirectUri(), client.get());

try {
URIBuilder uriBuilder = new URIBuilder(url);
Expand All @@ -124,8 +121,6 @@ public String confimAccess(Map<String, Object> model,
}
}

model.put("client", client);

// the authorization request already contains PDP filtered
// scopes among the request parameters due to the
// IamOAuth2RequestFactory.createAuthorizationRequest() object
Expand All @@ -135,14 +130,15 @@ public String confimAccess(Map<String, Object> model,

authRequest.setScope(scopes);

setModelForConsentPage(model, authRequest, authUser, client);
setModelForConsentPage(model, authRequest, authUser, client.get());

return APPROVE_AUTHZ_PAGE;
}

private void setModelForConsentPage(Map<String, Object> model, AuthorizationRequest authRequest,
Authentication authUser, ClientDetailsEntity client) {

model.put("client", client);
model.put("auth_request", authRequest);
model.put("redirect_uri", authRequest.getRedirectUri());
model.put("scopes", scopeService.fromStrings(authRequest.getScope()));
Expand Down
Loading

0 comments on commit 103aba3

Please sign in to comment.