Skip to content

feat(server): anonymous read access to /w3c/ and /oa/ when auth enabled #105

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
hairmare wants to merge 1 commit into
base: master
Choose a base branch
from
Open

feat(server): anonymous read access to /w3c/ and /oa/ when auth enabled #105

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 31 additions & 11 deletions elucidate-server/src/main/java/com/digirati/elucidate/infrastructure/config/AuthConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import com.digirati.elucidate.repository.security.GroupRepository;
import com.digirati.elucidate.repository.security.UserRepository;
import org.apache.commons.lang3.StringUtils;
import org.springframework.http.HttpMethod;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Qualifier;
import org.springframework.beans.factory.annotation.Value;
Expand All @@ -19,7 +20,6 @@
import org.springframework.security.config.annotation.method.configuration.EnableGlobalMethodSecurity;
import org.springframework.security.config.annotation.web.builders.HttpSecurity;
import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity;
import org.springframework.security.config.annotation.web.configurers.ExpressionUrlAuthorizationConfigurer;
import org.springframework.security.config.http.SessionCreationPolicy;
import org.springframework.security.jwt.crypto.sign.MacSigner;
import org.springframework.security.jwt.crypto.sign.RsaVerifier;
Expand Down Expand Up @@ -69,11 +69,17 @@ public class AuthConfig implements ResourceServerConfigurer {
private String uidProperties;

/**
* The public key used to verify a tokens signature.
* Is auth enabled?
*/
@Value("${auth.enabled:false}")
private boolean authEnabled;

/**
* Allow anonymous access to /w3c/* and /oa/* endpoints even when auth is enabled?
*/
@Value("${auth.anonReadAccess:false}")
private boolean anonReadAccess;

/**
* The URL scheme that will be used in the OAuth2 resource id.
*/
Expand Down Expand Up @@ -119,16 +125,30 @@ public void configure(ResourceServerSecurityConfigurer resources) throws Excepti

@Override
public void configure(HttpSecurity http) throws Exception {
ExpressionUrlAuthorizationConfigurer.AuthorizedUrl authorizationConfigurer = http
.sessionManagement().sessionCreationPolicy(SessionCreationPolicy.STATELESS)
.and()
.authorizeRequests()
.anyRequest();

if (authEnabled) {
authorizationConfigurer.authenticated();
HttpSecurity authorizationConfigurer = http
.sessionManagement()
.sessionCreationPolicy(SessionCreationPolicy.STATELESS)
.and();

if (authEnabled && anonReadAccess) {
authorizationConfigurer
.authorizeRequests()
.antMatchers(HttpMethod.GET, "/w3c/**", "/oa/**")
.permitAll()
.and()
.authorizeRequests()
.anyRequest()
.authenticated();
} else if (authEnabled) {
authorizationConfigurer
.authorizeRequests()
.anyRequest()
.authenticated();
} else {
authorizationConfigurer.permitAll();
authorizationConfigurer
.authorizeRequests()
.anyRequest()
.permitAll();
}
}

Expand Down
4 changes: 4 additions & 0 deletions ...va/com/digirati/elucidate/infrastructure/security/impl/JwtUserSecurityDetailsContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import org.springframework.security.core.GrantedAuthority;
import org.springframework.security.core.authority.SimpleGrantedAuthority;
import org.springframework.security.core.context.SecurityContextHolder;
import org.springframework.security.authentication.AnonymousAuthenticationToken;

import java.util.Collection;

Expand All @@ -20,6 +21,9 @@ public class JwtUserSecurityDetailsContext implements UserSecurityDetailsContext
@Override
public boolean isAuthorized(Permission operation, AbstractAnnotation annotation) {
Authentication auth = SecurityContextHolder.getContext().getAuthentication();
if (auth instanceof AnonymousAuthenticationToken) {
Copy link
Copy Markdown
Contributor

@garyttierney garyttierney Oct 30, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, the effect of this is that anonymous users are authorized for every action over an annotation. Is this what you're after? I think it's missing an operation == Permission.READ check.

Copy link
Copy Markdown
Contributor Author

@hairmare hairmare Oct 30, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was under the impression that the READ part already is taken care of because of the antMatcher only covering GET methods (only the permitAll() path creates a AnonymousAuthenticationToken):

elucidate-server/elucidate-server/src/main/java/com/digirati/elucidate/infrastructure/config/AuthConfig.java

Lines 135 to 141 in 041ab5e

.authorizeRequests()
.antMatchers(HttpMethod.GET, "/w3c/**", "/oa/**")
.permitAll()
.and()
.authorizeRequests()
.anyRequest()
.authenticated();

I'll take a closer look into doing more checking on the operation in JwtUserSecurityDetailsContext which would be more robust anyway.

Copy link
Copy Markdown
Contributor

@garyttierney garyttierney Nov 2, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, you're right. This is fine as is then. I don't think the test for the anonymous token would be any clearer without representing an anonymous user as a complete UserDetails instance.

I'm happy to merge this as is.

return true;
}
UserSecurityDetails details = (UserSecurityDetails) auth.getPrincipal();
Collection<? extends GrantedAuthority> roles = auth.getAuthorities();

Expand Down
1 change: 1 addition & 0 deletions elucidate-server/src/main/resources/eludicate-server.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ auth.enabled=false
auth.token.verifierType=secret
auth.token.verifierKey=
auth.token.uidProperties=sub,user_name
auth.anonReadAccess=false

# Generator to use when generating Security Group IDs
annotation.group.id.generator=com.digirati.elucidate.infrastructure.generator.impl.UUIDIDGeneratorImpl
Expand Down