Skip to content

Commit

Permalink
Bit of a kludge to work around what seems to be a problem with loggin…
Browse files Browse the repository at this point in the history
…g out not completely clearing the authentication, leaving the user as authenticated, but as an "anonymous" user, which means they really AREN'T authenticated, so the AuthenticationPrincipal ends up being null. This is either a bug or a misunderstanding of how logout works.

Added the Docker Compose module to automatically start the Postgres container when the application starts and removed the run file.

Added dependency for the Zalando logger to help with logging.

Trying to log more info about rejected URLs.
  • Loading branch information
tedyoung committed Dec 19, 2023
1 parent d15e2c1 commit 307a784
Show file tree
Hide file tree
Showing 11 changed files with 123 additions and 53 deletions.
33 changes: 0 additions & 33 deletions .run/postgres14.run.xml

This file was deleted.

10 changes: 10 additions & 0 deletions compose.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
services:
database:
image: 'postgres:14-alpine'
ports:
- '5432'
environment:
- 'POSTGRES_USER=postgres'
- 'POSTGRES_DB=postgres'
- 'POSTGRES_PASSWORD=password'
- "SPRING_PROFILES_ACTIVE=local"
14 changes: 13 additions & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,18 @@
<java.version>17</java.version>
</properties>
<dependencies>
<dependency>
<groupId>org.zalando</groupId>
<artifactId>logbook-spring-boot-starter</artifactId>
<version>3.7.2</version>
</dependency>

<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-docker-compose</artifactId>
<optional>true</optional>
</dependency>

<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-actuator</artifactId>
Expand Down Expand Up @@ -121,7 +133,7 @@
<dependency>
<groupId>com.tngtech.archunit</groupId>
<artifactId>archunit-junit5</artifactId>
<version>1.0.1</version>
<version>1.2.0</version>
<scope>test</scope>
</dependency>

Expand Down
51 changes: 40 additions & 11 deletions src/main/java/com/jitterted/mobreg/WebSecurityConfig.java
Original file line number Diff line number Diff line change
@@ -1,44 +1,73 @@
package com.jitterted.mobreg;

import com.jitterted.mobreg.adapter.in.web.member.MemberDeniedRedirectToUserOnboardingHandler;
import jakarta.servlet.http.HttpServletRequest;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
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.configuration.WebSecurityCustomizer;
import org.springframework.security.core.authority.mapping.GrantedAuthoritiesMapper;
import org.springframework.security.web.SecurityFilterChain;
import org.springframework.security.web.authentication.logout.HeaderWriterLogoutHandler;
import org.springframework.security.web.firewall.FirewalledRequest;
import org.springframework.security.web.firewall.RequestRejectedException;
import org.springframework.security.web.firewall.StrictHttpFirewall;
import org.springframework.security.web.header.writers.ClearSiteDataHeaderWriter;

import static org.springframework.security.web.header.writers.ClearSiteDataHeaderWriter.Directive.COOKIES;

@Configuration
@EnableWebSecurity
public class WebSecurityConfig {

private static final Logger LOGGER = LoggerFactory.getLogger(WebSecurityConfig.class);

private final GrantedAuthoritiesMapper userAuthoritiesMapper;

public WebSecurityConfig(GrantedAuthoritiesMapper userAuthoritiesMapper) {
this.userAuthoritiesMapper = userAuthoritiesMapper;
}

@Bean
SecurityFilterChain filterChain(HttpSecurity http) throws Exception {
public WebSecurityCustomizer webSecurityCustomizer() {
StrictHttpFirewall firewall = new StrictHttpFirewall() {
@Override
public FirewalledRequest getFirewalledRequest(HttpServletRequest request) throws RequestRejectedException {
try {
return super.getFirewalledRequest(request);
} catch(RequestRejectedException rre) {
LOGGER.info("HttpRequest rejected URL: {}", request.getRequestURI());
throw rre;
}
}
};
// firewall.setAllowBackSlash(true);
// firewall.setAllowUrlEncodedSlash(true);
// firewall.setAllowUrlEncodedDoubleSlash(true);
return (web) -> web.httpFirewall(firewall);
}

@Bean
public SecurityFilterChain filterChain(HttpSecurity http) throws Exception {
// @formatter:off
http
.authorizeHttpRequests(requests -> requests
.requestMatchers("/**", "/error")
.permitAll()
.requestMatchers("/user/**", "/invite")
.hasAuthority("ROLE_USER")
.requestMatchers("/admin/**")
.hasAuthority("ROLE_ADMIN")
.requestMatchers("/member/**")
.hasAuthority("ROLE_MEMBER"))
.requestMatchers("/**", "/error").permitAll()
.requestMatchers("/user/**", "/invite").hasAuthority("ROLE_USER")
.requestMatchers("/admin/**").hasAuthority("ROLE_ADMIN")
.requestMatchers("/member/**").hasAuthority("ROLE_MEMBER"))
.exceptionHandling(handling -> handling
.accessDeniedHandler(new MemberDeniedRedirectToUserOnboardingHandler()))
.logout(logout -> logout
.addLogoutHandler(new HeaderWriterLogoutHandler(new ClearSiteDataHeaderWriter(COOKIES)))
.logoutSuccessUrl("/")
.permitAll()
.deleteCookies("JSESSIONID")
.clearAuthentication(true)
.invalidateHttpSession(true))
.oauth2Login(login -> login
.oauth2Login(oauth2 -> oauth2
.userInfoEndpoint(endpoint -> endpoint
.userAuthoritiesMapper(userAuthoritiesMapper)));
return http.build();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
package com.jitterted.mobreg.adapter.in.web;

import org.springframework.security.access.AccessDeniedException;
import org.springframework.security.core.AuthenticatedPrincipal;
import org.springframework.security.core.annotation.AuthenticationPrincipal;
import org.springframework.security.core.annotation.CurrentSecurityContext;
import org.springframework.security.core.context.SecurityContext;
import org.springframework.stereotype.Controller;
import org.springframework.ui.Model;
import org.springframework.web.bind.annotation.GetMapping;
Expand All @@ -10,7 +13,10 @@
public class WelcomeController {

@GetMapping("/member")
public String memberHome() {
public String memberHome(@CurrentSecurityContext SecurityContext context) {
if (context.getAuthentication().getName().equalsIgnoreCase("anonymousUser")) {
throw new AccessDeniedException("Access Denied for Anonymous User");
}
return "redirect:/member/register";
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,11 @@
import com.jitterted.mobreg.domain.EnsembleId;
import com.jitterted.mobreg.domain.Member;
import org.springframework.http.HttpStatus;
import org.springframework.security.access.AccessDeniedException;
import org.springframework.security.core.AuthenticatedPrincipal;
import org.springframework.security.core.annotation.AuthenticationPrincipal;
import org.springframework.security.core.annotation.CurrentSecurityContext;
import org.springframework.security.core.context.SecurityContext;
import org.springframework.security.oauth2.core.user.OAuth2User;
import org.springframework.stereotype.Controller;
import org.springframework.ui.Model;
Expand All @@ -34,15 +37,20 @@ public AdminDashboardController(EnsembleService ensembleService,
}

@GetMapping("/dashboard")
public String dashboardView(Model model, @AuthenticationPrincipal AuthenticatedPrincipal principal) {
public String dashboardView(Model model,
@AuthenticationPrincipal AuthenticatedPrincipal principal,
@CurrentSecurityContext SecurityContext context) {
if (principal instanceof OAuth2User oAuth2User) {
String username = oAuth2User.getAttribute("login");
Member member = memberService.findByGithubUsername(username);
model.addAttribute("username", username); // Member.githubUsername
model.addAttribute("name", member.firstName());
model.addAttribute("github_id", oAuth2User.getAttribute("id"));
} else {
throw new IllegalStateException("Not an OAuth2User");
if (context.getAuthentication().getName().equalsIgnoreCase("anonymousUser")) {
throw new AccessDeniedException("Access Denied for Anonymous User");
}
throw new IllegalStateException("AuthenticationPrincipal is not an OAuth2User: " + principal);
}
List<Ensemble> ensembles = ensembleService.allEnsemblesByDateTimeDescending();
List<EnsembleSummaryView> ensembleSummaryViews = EnsembleSummaryView.from(ensembles);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,13 @@
import com.jitterted.mobreg.domain.EnsembleId;
import com.jitterted.mobreg.domain.Member;
import com.jitterted.mobreg.domain.MemberId;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.security.access.AccessDeniedException;
import org.springframework.security.core.AuthenticatedPrincipal;
import org.springframework.security.core.annotation.AuthenticationPrincipal;
import org.springframework.security.core.annotation.CurrentSecurityContext;
import org.springframework.security.core.context.SecurityContext;
import org.springframework.stereotype.Controller;
import org.springframework.ui.Model;
import org.springframework.web.bind.annotation.GetMapping;
Expand All @@ -18,6 +23,8 @@
@Controller
public class MemberController {

private static final Logger LOGGER = LoggerFactory.getLogger(MemberController.class);

private final EnsembleService ensembleService;
private final MemberLookup memberLookup;
private final MemberService memberService;
Expand All @@ -29,7 +36,13 @@ public MemberController(EnsembleService ensembleService, MemberService memberSer
}

@GetMapping("/member/register")
public String showEnsemblesForUser(Model model, @AuthenticationPrincipal AuthenticatedPrincipal principal) {
public String showEnsemblesForUser(Model model,
@AuthenticationPrincipal AuthenticatedPrincipal principal,
@CurrentSecurityContext SecurityContext context) {
if (context.getAuthentication().getName().equalsIgnoreCase("anonymousUser")) {
throw new AccessDeniedException("Access Denied for Anonymous User");
}

Member member = memberLookup.findMemberBy(principal);
model.addAttribute("githubUsername", member.githubUsername());
model.addAttribute("firstName", member.firstName());
Expand Down
3 changes: 3 additions & 0 deletions src/main/resources/application-local.properties
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
spring.security.oauth2.client.registration.github.clientId=${github.oauth2.local.clientId}
spring.security.oauth2.client.registration.github.clientSecret=${github.oauth2.local.clientSecret}

# Turn on for super detailed security logging
#logging.level.org.springframework.security.web=TRACE

# Local (Docker container) PostgreSQL (not a Testcontainer)
spring.datasource.username=postgres
spring.datasource.password=password
Expand Down
6 changes: 4 additions & 2 deletions src/main/resources/application.properties
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,14 @@ zoom.client.secret=${zoom.client.secret}
#logging.level.org.springframework.jdbc.core=TRACE
logging.level.com.jitterted.mobreg.adapter.in.web.GitHubGrantedAuthoritiesMapper=DEBUG
logging.level.com.jitterted.mobreg.adapter.out.zoom=DEBUG
logging.level.org.springframework.web=DEBUG
logging.level.org.springframework.security.web=DEBUG

management.endpoints.web.exposure.include=health,info,metrics

app.version=@project.version@
app.name=@project.name@
app.build.timestamp=@maven.build.timestamp@

# Don't need to set the active profile, it should be an environment variable
#spring.profiles.active=railway
# Set profile to local, should be overridden by environment when deployed
spring.profiles.active=local
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,18 @@
import com.jitterted.mobreg.domain.ZonedDateTimeFactory;
import org.jetbrains.annotations.NotNull;
import org.junit.jupiter.api.Test;
import org.springframework.security.core.authority.SimpleGrantedAuthority;
import org.springframework.security.core.context.SecurityContextImpl;
import org.springframework.security.oauth2.client.authentication.OAuth2AuthenticationToken;
import org.springframework.security.oauth2.core.user.OAuth2User;
import org.springframework.ui.ConcurrentModel;
import org.springframework.ui.Model;

import java.time.ZoneId;
import java.time.ZoneOffset;
import java.time.ZonedDateTime;
import java.util.List;
import java.util.Set;

import static org.assertj.core.api.Assertions.*;

Expand All @@ -43,7 +48,11 @@ void givenOneEnsembleResultsInEnsembleInViewModel() throws Exception {
AdminDashboardController adminDashboardController = new AdminDashboardController(ensembleService, memberService);

Model model = new ConcurrentModel();
adminDashboardController.dashboardView(model, OAuth2UserFactory.createOAuth2UserWithMemberRole("tedyoung", "ROLE_MEMBER"));
OAuth2User oAuth2UserWithMemberRole = OAuth2UserFactory.createOAuth2UserWithMemberRole("tedyoung", "ROLE_MEMBER");
SecurityContextImpl securityContext = new SecurityContextImpl(new OAuth2AuthenticationToken(oAuth2UserWithMemberRole,
Set.of(new SimpleGrantedAuthority("ROLE_MEMBER")),
"github"));
adminDashboardController.dashboardView(model, oAuth2UserWithMemberRole, securityContext);

List<EnsembleSummaryView> ensembleSummaryViews = (List<EnsembleSummaryView>) model.getAttribute("ensembles");
assertThat(ensembleSummaryViews)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,15 @@
import com.jitterted.mobreg.domain.MemberStatus;
import org.jetbrains.annotations.NotNull;
import org.junit.jupiter.api.Test;
import org.springframework.security.core.authority.SimpleGrantedAuthority;
import org.springframework.security.core.context.SecurityContextImpl;
import org.springframework.security.oauth2.client.authentication.OAuth2AuthenticationToken;
import org.springframework.security.oauth2.core.user.OAuth2User;
import org.springframework.ui.ConcurrentModel;
import org.springframework.ui.Model;

import java.time.ZonedDateTime;
import java.util.Set;

import static org.assertj.core.api.Assertions.*;

Expand All @@ -42,7 +47,13 @@ void ensembleFormContainsMemberIdForOAuth2User() throws Exception {
MemberController memberController = new MemberController(ensembleService, memberService);

Model model = new ConcurrentModel();
memberController.showEnsemblesForUser(model, OAuth2UserFactory.createOAuth2UserWithMemberRole("ghuser", "ROLE_MEMBER"));
OAuth2User oAuth2UserWithMemberRole = OAuth2UserFactory.createOAuth2UserWithMemberRole("ghuser", "ROLE_MEMBER");
SecurityContextImpl securityContext = new SecurityContextImpl(new OAuth2AuthenticationToken(oAuth2UserWithMemberRole,
Set.of(new SimpleGrantedAuthority("ROLE_MEMBER")),
"github"));
memberController.showEnsemblesForUser(model,
oAuth2UserWithMemberRole,
securityContext);

assertThat((String) model.getAttribute("firstName"))
.isEqualTo("name");
Expand Down

0 comments on commit 307a784

Please sign in to comment.