Skip to content

AB#72068 Allow requests to admin endpoint #194

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

Merged
buckett merged 2 commits into from
Jan 13, 2026
Merged

AB#72068 Allow requests to admin endpoint #194

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
67bcf18
AB#72068 Allow requests to admin endpoint
buckett Dec 18, 2025
71fa44b
AB#72068 We don't need to allow credentials
buckett Jan 6, 2026
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
22 changes: 22 additions & 0 deletions src/main/java/uk/ac/ox/ctl/admin/AdminProperties.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package uk.ac.ox.ctl.admin;

import org.springframework.boot.context.properties.ConfigurationProperties;
import org.springframework.context.annotation.Configuration;

import java.util.Collections;
import java.util.List;

@Configuration
@ConfigurationProperties(prefix = "admin")
public class AdminProperties {

Comment on lines +10 to +12
Copy link

Copilot AI Jan 2, 2026

Choose a reason for hiding this comment

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

The AdminProperties class lacks documentation explaining its purpose and usage. As this is a configuration class that controls CORS behavior for admin endpoints, it should include class-level and field-level documentation explaining what the corsOrigins property does and providing examples of proper configuration values.

Suggested change
@ConfigurationProperties(prefix = "admin")
public class AdminProperties {
@ConfigurationProperties(prefix = "admin")
/**
* Configuration properties for admin endpoints.
* <p>
* This class is bound to properties with the {@code admin} prefix
* (for example {@code admin.cors-origins}) and is used to control
* behaviour that is specific to administrative APIs and UIs.
* <p>
* Typical configuration examples (in {@code application.yml}):
* <pre>{@code
* admin:
* cors-origins:
* - "https://admin.example.com"
* - "https://ops.example.org"
* }</pre>
*/
public class AdminProperties {
/**
* List of allowed CORS origins for admin endpoints.
* <p>
* Each value must be a valid origin, such as
* {@code https://admin.example.com}. An empty list
* means that no additional CORS origins are configured
* for admin endpoints.
* <p>
* Example (in {@code application.properties}):
* <pre>{@code
* admin.cors-origins=https://admin.example.com,https://ops.example.org
* }</pre>
*/

Copilot uses AI. Check for mistakes.
private List<String> corsOrigins = Collections.emptyList();

public List<String> getCorsOrigins() {
return corsOrigins;
}

public void setCorsOrigins(List<String> corsOrigins) {
this.corsOrigins = corsOrigins;
}
}
26 changes: 25 additions & 1 deletion src/main/java/uk/ac/ox/ctl/admin/AdminWebSecurity.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@
import org.springframework.security.provisioning.InMemoryUserDetailsManager;
import org.springframework.security.web.SecurityFilterChain;
import org.springframework.util.StringUtils;
import org.springframework.web.cors.CorsConfiguration;
import org.springframework.web.cors.CorsConfigurationSource;
import org.springframework.web.cors.UrlBasedCorsConfigurationSource;

import java.util.List;
import java.util.regex.Pattern;
Expand Down Expand Up @@ -53,12 +56,33 @@ private String getOrDeducePassword(SecurityProperties.User user, PasswordEncoder
}
return NOOP_PASSWORD_PREFIX + password;
}

Copy link

Copilot AI Jan 2, 2026

Choose a reason for hiding this comment

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

The corsConfigurationSource method lacks documentation explaining its purpose, parameters, and how it integrates with the security configuration. Consider adding a Javadoc comment to explain the CORS configuration behavior, especially the relationship with AdminProperties and the security implications of the configuration choices.

Suggested change
/**
* Builds a {@link CorsConfigurationSource} for the admin endpoints.
* <p>
* The allowed origins are taken from {@link AdminProperties#getCorsOrigins()}.
* For each configured origin, a corresponding allowed origin is added to the
* CORS configuration. If the special {@link CorsConfiguration#ALL} value
* ({@code "*"}) is present, a warning is logged as this is generally not
* recommended for production environments.
* <p>
* The resulting configuration:
* <ul>
* <li>applies only to {@code /admin/**} paths,</li>
* <li>allows credentials,</li>
* <li>allows all headers, and</li>
* <li>allows all HTTP methods.</li>
* </ul>
* This method is used by {@link #adminConfiguration(HttpSecurity, AdminProperties)}
* to enable CORS handling for the admin security filter chain.
*
* @param adminProperties configuration properties providing the list of
* allowed CORS origins for the admin endpoints
* @return a {@link CorsConfigurationSource} to be used by Spring Security
* when processing CORS requests to {@code /admin/**}
*/

Copilot uses AI. Check for mistakes.
public CorsConfigurationSource corsConfigurationSource(AdminProperties adminProperties) {
Copy link

Copilot AI Jan 2, 2026

Choose a reason for hiding this comment

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

The corsConfigurationSource method deviates from the established pattern in WebSecurityConfiguration where a similar method is annotated with @bean. For consistency and to follow the existing codebase patterns, consider making this a @bean method and giving it a distinctive name (e.g., "adminCorsConfigurationSource") to avoid bean name conflicts.

Copilot uses AI. Check for mistakes.
if (adminProperties.getCorsOrigins().contains(CorsConfiguration.ALL)) {
log.warn("CORS allowed origins is set to '*', this is not recommended for production environments.");
}
if (adminProperties.getCorsOrigins().isEmpty()) {
log.info("No Admin CORS allowed origins configured.");
} else {
log.info("Admin CORS allowed origins: {}", String.join(", ", adminProperties.getCorsOrigins()));
}
CorsConfiguration config = new CorsConfiguration();
config.addAllowedHeader(CorsConfiguration.ALL);
config.addAllowedMethod(CorsConfiguration.ALL);
for (String origin : adminProperties.getCorsOrigins()) {
config.addAllowedOrigin(origin);
}
UrlBasedCorsConfigurationSource source = new UrlBasedCorsConfigurationSource();
source.registerCorsConfiguration("/admin/**", config);
return source;
}

@Bean
@Order(1)
public SecurityFilterChain adminConfiguration(HttpSecurity http) throws Exception {
public SecurityFilterChain adminConfiguration(HttpSecurity http, AdminProperties adminProperties) throws Exception {
return http.securityMatcher("/admin/**")
.csrf(AbstractHttpConfigurer::disable)
.cors(cors -> cors.configurationSource(corsConfigurationSource(adminProperties)))
.sessionManagement(session -> session.sessionCreationPolicy(SessionCreationPolicy.STATELESS))
.httpBasic(withDefaults())
.authorizeHttpRequests(authorize -> authorize.anyRequest().hasRole("admin"))
Expand Down
2 changes: 1 addition & 1 deletion src/test/java/uk/ac/ox/ctl/admin/AdminControllerWebTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.*;

@WebMvcTest(properties = {"spring.security.user.name=user", "spring.security.user.password=pass1234"}, controllers = AdminController.class, excludeFilters = @ComponentScan.Filter(type = FilterType.REGEX, pattern = "uk\\.ac\\.ox\\.ctl\\.(canvasproxy|ltiauth)\\..*"))
@Import({AdminWebSecurity.class})
@Import({AdminWebSecurity.class, AdminProperties.class})
Copy link

Copilot AI Jan 2, 2026

Choose a reason for hiding this comment

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

The newly added CORS configuration for admin endpoints lacks test coverage. Since this file contains comprehensive tests for other AdminController functionality, the CORS behavior should also be tested. Consider adding tests to verify that CORS headers are properly set when configured origins are provided, and that CORS requests are rejected when origins don't match the configuration.

Copilot uses AI. Check for mistakes.
@ImportAutoConfiguration(exclude = {
OAuth2ClientAutoConfiguration.class
})
Expand Down