Skip to content

Commit

Permalink
Merged in shibui-2646 (pull request #678)
Browse files Browse the repository at this point in the history 
SHIBUI-2646: MDQ fixes for trailing slashes

Approved-by: Sean Porth
  • Loading branch information
@chasegawa @sporth
chasegawa authored and sporth committed Jan 8, 2025
2 parents 400d871 + fa8ae2c commit d35437a
Show file tree
Hide file tree
Showing 6 changed files with 17 additions and 101 deletions.
11 changes: 8 additions & 3 deletions .../main/java/edu/internet2/tier/shibboleth/admin/ui/configuration/SpringSecurityConfig.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,8 @@ public SecurityFilterChain filterChain(HttpSecurity http) throws Exception {
.authorizeHttpRequests()
.requestMatchers(new AntPathRequestMatcher("/unsecured/**/*"),
new AntPathRequestMatcher("/entities*"),
new AntPathRequestMatcher("/entities/**/*"),
new AntPathRequestMatcher("/entities/*"),
new AntPathRequestMatcher("/entities/**"),
new AntPathRequestMatcher("/actuator/**"),
new AntPathRequestMatcher("/api/beacon/send")).permitAll()
.anyRequest().hasAnyRole(acceptedAuthenticationRoles)
Expand Down Expand Up @@ -158,7 +159,9 @@ public InMemoryUserDetailsManager userDetailsManager() {
@Profile("!no-auth")
public WebSecurityCustomizer webSecurityCustomizer() {
return (web) -> web.ignoring().requestMatchers(new AntPathRequestMatcher("/unsecured/**/*"),
new AntPathRequestMatcher("/entities/**/*"),
new AntPathRequestMatcher("/entities*"),
new AntPathRequestMatcher("/entities/*"),
new AntPathRequestMatcher("/entities/**"),
new AntPathRequestMatcher("/favicon.ico"),
new AntPathRequestMatcher("/assets/**/*.png"),
new AntPathRequestMatcher("/static/**/*"),
Expand Down Expand Up @@ -188,7 +191,9 @@ public WebSecurityCustomizer noAuthWebSecurityCustomizer() {
}

private HttpFirewall defaultFirewall() {
return new DefaultHttpFirewall();
DefaultHttpFirewall defaultFirewall = new DefaultHttpFirewall();
defaultFirewall.setAllowUrlEncodedSlash(true);
return defaultFirewall;
}

class SimpleAuthenticationProvider implements AuthenticationProvider {
Expand Down
19 changes: 2 additions & 17 deletions ...d/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/EntitiesController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public class EntitiesController {
@Autowired
private EntityDescriptorRepository entityDescriptorRepository;

@RequestMapping(value = "/entities", produces = "application/xml")
@RequestMapping(value = {"/entities", "/entities/"}, produces = {"application/xml", "application/xml"})
@Operation(description = "Endpoint based on the MDQ spec to return all entity's information. see: https://spaces.at.internet2.edu/display/MDQ/Metadata+Query+Protocol",
summary = "Return all the entities from the entity's id", method = "GET")
@Transactional(readOnly = true)
Expand All @@ -72,28 +72,13 @@ public ResponseEntity<?> getAllXml() throws MarshallingException, ResolverExcept
return new ResponseEntity<>("<?xml version=\"1.0\" encoding=\"UTF-8\"?>" + xmlDeclarationClean, new HttpHeaders(), HttpStatus.OK);
}

@RequestMapping(value = "/entities/{entityId:.*}")
@Operation(description = "Endpoint based on the MDQ spec to return a single entity's information. see: https://spaces.at.internet2.edu/display/MDQ/Metadata+Query+Protocol",
summary = "Return a single entity from the entity's id", method = "GET")
@Transactional(readOnly = true)
public ResponseEntity<?> getOne(final @PathVariable String entityId, HttpServletRequest request) throws UnsupportedEncodingException, ResolverException {
EntityDescriptor entityDescriptor = this.getEntityDescriptor(entityId);
if (entityDescriptor == null) {
return ResponseEntity.notFound().build();
}
EntityDescriptorRepresentation entityDescriptorRepresentation = entityDescriptorService.createRepresentationFromDescriptor(entityDescriptor);
HttpHeaders headers = new HttpHeaders();
headers.set("Last-Modified", formatModifiedDate(entityDescriptorRepresentation));
return new ResponseEntity<>(entityDescriptorRepresentation, headers, HttpStatus.OK);
}

private String formatModifiedDate(EntityDescriptorRepresentation entityDescriptorRepresentation) {
Instant instant = entityDescriptorRepresentation.getModifiedDateAsDate().toInstant(ZoneOffset.UTC);
Date date = Date.from(instant);
return DateUtils.formatDate(date, DateUtils.PATTERN_RFC1123);
}

@RequestMapping(value = "/entities/{entityId:.*}", produces = "application/xml")
@RequestMapping(value = {"/entities/{entityId:.*}", "/entities/{entityId:.*}/"}, produces = {"application/xml", "application/xml"})
@Operation(description = "Endpoint based on the MDQ spec to return a single entity's information. see: https://spaces.at.internet2.edu/display/MDQ/Metadata+Query+Protocol",
summary = "Return a single entity from the entity's id", method = "GET")
@Transactional(readOnly = true)
Expand Down
44 changes: 0 additions & 44 deletions ...u/internet2/tier/shibboleth/admin/ui/controller/EntitiesControllerIntegrationTests.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,50 +99,6 @@ class EntitiesControllerIntegrationTests extends AbstractBaseDataJpaTest {
}
}

@WithMockAdmin
def 'GET /entities/{resourceId} existing'() {
given:
def entityDescriptorOne = new EntityDescriptor(resourceId: 'uuid-1', entityID: 'eid1', serviceProviderName: 'sp1', serviceEnabled: true, idOfOwner: "admingroup")
entityDescriptorRepository.save(entityDescriptorOne)
entityManager.flush()
entityManager.clear()

when:
def result = mockMvc.perform(get("/entities/eid1"))

then:
result.andExpect(status().isOk())
.andExpect(jsonPath("\$.entityId").value("eid1"))
.andExpect(jsonPath("\$.serviceProviderName").value("sp1"))
.andExpect(jsonPath("\$.serviceEnabled").value(true))
.andExpect(jsonPath("\$.idOfOwner").value("admingroup"))
}

@WithMockUser(value = "someUser", roles = ["USER"])
def 'GET /entities/{resourceId} existing, validate group access'() {
given:
Group g = userService.getCurrentUserGroup()

def entityDescriptorOne = new EntityDescriptor(resourceId: 'uuid-1', entityID: 'eid1', serviceProviderName: 'sp1', serviceEnabled: true, idOfOwner: "someUser")
def entityDescriptorTwo = new EntityDescriptor(resourceId: 'uuid-2', entityID: 'eid2', serviceProviderName: 'sp2', serviceEnabled: false, idOfOwner: Group.ADMIN_GROUP.getOwnerId())

entityDescriptorRepository.saveAndFlush(entityDescriptorOne)
entityDescriptorRepository.saveAndFlush(entityDescriptorTwo)

ownershipRepository.saveAndFlush(new Ownership(g, entityDescriptorOne))
ownershipRepository.saveAndFlush(new Ownership(Group.ADMIN_GROUP, entityDescriptorTwo))

when:
def result = mockMvc.perform(get("/entities/eid1"))

then:
result.andExpect(status().isOk())
.andExpect(jsonPath("\$.entityId").value("eid1"))
.andExpect(jsonPath("\$.serviceProviderName").value("sp1"))
.andExpect(jsonPath("\$.serviceEnabled").value(true))
.andExpect(jsonPath("\$.idOfOwner").value("someUser"))
}

@WithMockUser(value = "someUser", roles = ["USER"])
def 'GET /entities/{resourceId} existing, owned by some other user'() {
when:
Expand Down
34 changes: 0 additions & 34 deletions ...t/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/EntitiesControllerTests.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,40 +94,6 @@ class EntitiesControllerTests extends AbstractBaseDataJpaTest {
result.andExpect(status().isNotFound())
}

def 'GET /api/entities/http%3A%2F%2Ftest.scaldingspoon.org%2Ftest1'() {
when:
def result = mockMvc.perform(get('/entities/http%3A%2F%2Ftest.scaldingspoon.org%2Ftest1'))

then:
// Response headers section 2.5
// from the spec https://www.ietf.org/archive/id/draft-young-md-query-14.txt
result.andExpect(status().isOk())
.andExpect(header().exists(HttpHeaders.CONTENT_TYPE)) // MUST HAVE
// .andExpect(header().exists(HttpHeaders.CONTENT_LENGTH)) // SHOULD HAVE - should end up from etag filter, so skipped for test
// .andExpect(header().exists(HttpHeaders.CACHE_CONTROL)) // SHOULD HAVE - should be included by Spring Security
// .andExpect(header().exists(HttpHeaders.ETAG)) // MUST HAVE - is done by filter, so skipped for test
.andExpect(header().exists(HttpHeaders.LAST_MODIFIED))
.andExpect(content().contentType(MediaType.APPLICATION_JSON))
.andExpect(jsonPath('$.entityId', is("http://test.scaldingspoon.org/test1")))
}

def 'GET /entities/http%3A%2F%2Ftest.scaldingspoon.org%2Ftest1'() {
when:
def result = mockMvc.perform(get('/entities/http%3A%2F%2Ftest.scaldingspoon.org%2Ftest1'))

then:
// Response headers section 2.5
// from the spec https://www.ietf.org/archive/id/draft-young-md-query-14.txt
result.andExpect(status().isOk())
.andExpect(header().exists(HttpHeaders.CONTENT_TYPE)) // MUST HAVE
// .andExpect(header().exists(HttpHeaders.CONTENT_LENGTH)) // SHOULD HAVE - should end up from etag filter, so skipped for test
// .andExpect(header().exists(HttpHeaders.CACHE_CONTROL)) // SHOULD HAVE - should be included by Spring Security
// .andExpect(header().exists(HttpHeaders.ETAG)) // MUST HAVE - is done by filter, so skipped for test
.andExpect(header().exists(HttpHeaders.LAST_MODIFIED))
.andExpect(content().contentType(MediaType.APPLICATION_JSON))
.andExpect(jsonPath('$.entityId').value("http://test.scaldingspoon.org/test1"))
}

def 'GET /api/entities/http%3A%2F%2Ftest.scaldingspoon.org%2Ftest1 XML'() {
given:
def expectedBody = '''<?xml version="1.0" encoding="UTF-8"?>
Expand Down
2 changes: 1 addition & 1 deletion gradle.properties
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
name=shibui
group=edu.internet2.tier.shibboleth.admin.ui
version=2.0.5
version=2.0.6-SNAPSHOT

### library versions ###
## As of 2-23-23
Expand Down
8 changes: 6 additions & 2 deletions pac4j-module/src/main/java/net/unicon/shibui/pac4j/Pac4jSpringSecurityConfig.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,9 @@ public AuditorAware<String> pac4jAuditorAware() {
@Bean
public SecurityFilterChain filterChain(HttpSecurity http) throws Exception {
http.authorizeHttpRequests().requestMatchers(new AntPathRequestMatcher("/unsecured/**/*"),
new AntPathRequestMatcher("/entities/**/*"),
new AntPathRequestMatcher("/entities*"),
new AntPathRequestMatcher("/entities/*"),
new AntPathRequestMatcher("/entities/**"),
new AntPathRequestMatcher("/favicon.ico"),
new AntPathRequestMatcher("/assets/**/*.png"),
new AntPathRequestMatcher("/static/**/*"),
Expand Down Expand Up @@ -127,7 +129,9 @@ public WebSecurityCustomizer webSecurityCustomizer() {
firewall.setAllowSemicolon(true);

return (web) -> web.ignoring().requestMatchers(new AntPathRequestMatcher("/unsecured/**/*"),
new AntPathRequestMatcher("/entities/**/*"),
new AntPathRequestMatcher("/entities*"),
new AntPathRequestMatcher("/entities/*"),
new AntPathRequestMatcher("/entities/**"),
new AntPathRequestMatcher("/favicon.ico"),
new AntPathRequestMatcher("/assets/**/*.png"),
new AntPathRequestMatcher("/static/**/*"),
Expand Down

0 comments on commit d35437a

Please sign in to comment.