-
Notifications
You must be signed in to change notification settings - Fork 0
23 create security config file Add API key management, role-based security, and Google OIDC integration #31
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
5f7c911
9a803fb
ea3d752
d94aa78
51b5a84
35ec3ee
35c4ca2
036eb0c
5e12d76
7f38b60
2068722
f559892
00ad31d
ac9cd7f
c02518e
83b6c9c
73c243d
22a64cb
fa77ec5
05b17b5
a6ee85c
7b7bd06
8fd974a
c54730e
b581bc0
71a3d69
b49385e
ee65d20
902f001
0a14e62
1e5d67b
0072996
75dde55
4a21fd9
2eced2a
79f874a
573ca4e
6ef9dcf
b6ffe92
28fc5a9
21e22b4
0ba8001
20f9454
3e8b927
a2661ba
e081e18
95f5b37
5c0ff5a
f39b682
46f51bb
54368e1
448f45d
77d8972
3f8bd11
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -31,4 +31,4 @@ build/ | |
|
|
||
| ### VS Code ### | ||
| .vscode/ | ||
| .env | ||
| .env | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,114 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| package org.storkforge.barkr.api.controller; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| import org.springframework.format.annotation.DateTimeFormat; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| import org.springframework.security.access.prepost.PreAuthorize; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| import org.springframework.security.core.annotation.AuthenticationPrincipal; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| import org.springframework.security.oauth2.core.oidc.user.OidcUser; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| import org.springframework.stereotype.Controller; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| import org.springframework.ui.Model; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| import org.springframework.web.bind.annotation.*; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| import org.springframework.web.servlet.mvc.support.RedirectAttributes; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| import org.storkforge.barkr.domain.IssuedApiKeyService; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| import org.storkforge.barkr.dto.apiKeyDto.GenerateApiKeyRequest; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| import org.storkforge.barkr.dto.apiKeyDto.ResponseApiKeyOnce; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| import org.storkforge.barkr.dto.apiKeyDto.UpdateApiKey; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| import org.storkforge.barkr.mapper.ApiKeyMapper; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.security.InvalidKeyException; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.security.NoSuchAlgorithmException; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.time.LocalDateTime; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.time.format.DateTimeFormatter; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.time.temporal.ChronoUnit; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.util.UUID; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| @Controller | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| @RequestMapping("/apikeys") | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| public class ApiKeyController { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| private final IssuedApiKeyService issuedApiKeyService; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| public ApiKeyController(IssuedApiKeyService issuedApiKeyService) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| this.issuedApiKeyService = issuedApiKeyService; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| @GetMapping("/apikeyform") | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| @PreAuthorize("hasRole('USER')") | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| public String showApiKeyForm(Model model) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| String now = LocalDateTime.now().truncatedTo(ChronoUnit.MINUTES) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| .format(DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm")); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| model.addAttribute("now", now); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| return "apikeys"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| @GetMapping("/result") | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| @PreAuthorize("hasRole('USER')") | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| public String showGeneratedApiKey(@ModelAttribute("response") ResponseApiKeyOnce response, Model model) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (response == null || response.value() == null) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| return "redirect:/apikeys/apikeyform"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| model.addAttribute("response", response); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| return "apikeys/result"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| @PostMapping("/generate") | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| @PreAuthorize("hasRole('USER')") | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| public String generateApiKey( | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| @RequestParam String apiKeyName, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| @RequestParam(required = false) @DateTimeFormat(iso = DateTimeFormat.ISO.DATE_TIME | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) LocalDateTime expiresAt, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| RedirectAttributes redirectAttributes | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) throws NoSuchAlgorithmException, InvalidKeyException { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| GenerateApiKeyRequest inputRequest = new GenerateApiKeyRequest(apiKeyName, expiresAt); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| GenerateApiKeyRequest request = ApiKeyMapper.normalizeExpiresAt(inputRequest); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| String apiKey; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| String hashedApiKey; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| do{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| apiKey = issuedApiKeyService.generateRawApiKey(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| hashedApiKey = issuedApiKeyService.hashedApiKey(apiKey); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } while (issuedApiKeyService.apiKeyExists(hashedApiKey)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| issuedApiKeyService.apiKeyGenerate(request, hashedApiKey); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| ResponseApiKeyOnce response = new ResponseApiKeyOnce( | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| "API-KEY", apiKey, "Please store these credentials safely"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| redirectAttributes.addFlashAttribute("response", response); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| return "redirect:/apikeys/result"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| @GetMapping("/mykeys") | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| public String myKeys(Model model , @AuthenticationPrincipal OidcUser user) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| var currentUser = issuedApiKeyService.findByGoogleOidc2Id(user.getName()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| var keys = issuedApiKeyService.allApiKeys(user.getName()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| model.addAttribute("account", currentUser.get().getAccount()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| model.addAttribute("keys", keys.apiKeys()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| return "apikeys/mykeys"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+86
to
+94
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Add @PreAuthorize annotation for consistency The @GetMapping("/mykeys")
+ @PreAuthorize("hasRole('USER')")
public String myKeys(Model model , @AuthenticationPrincipal OidcUser user) {📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| @PostMapping("/mykeys/revoke") | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| public String revokeKey(@RequestParam String referenceId, @AuthenticationPrincipal OidcUser user) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (issuedApiKeyService.isValidUuid(UUID.fromString(referenceId), user.getName())) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| var update = new UpdateApiKey(UUID.fromString(referenceId),null, true, null); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| issuedApiKeyService.updateApiKey(update); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+98
to
+101
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Add error handling for UUID validation and provide user feedback The method silently fails if the UUID is invalid. Consider adding proper error handling and redirecting with an error message if validation fails. @PostMapping("/mykeys/revoke")
public String revokeKey(@RequestParam String referenceId, @AuthenticationPrincipal OidcUser user, RedirectAttributes redirectAttributes) {
- if (issuedApiKeyService.isValidUuid(UUID.fromString(referenceId), user.getName())) {
- var update = new UpdateApiKey(UUID.fromString(referenceId),null, true, null);
- issuedApiKeyService.updateApiKey(update);
- }
+ try {
+ UUID uuid = UUID.fromString(referenceId);
+ if (issuedApiKeyService.isValidUuid(uuid, user.getName())) {
+ var update = new UpdateApiKey(uuid, null, true, null);
+ issuedApiKeyService.updateApiKey(update);
+ redirectAttributes.addFlashAttribute("success", "API key successfully revoked");
+ } else {
+ redirectAttributes.addFlashAttribute("error", "You don't have permission to revoke this API key");
+ }
+ } catch (IllegalArgumentException e) {
+ redirectAttributes.addFlashAttribute("error", "Invalid API key reference");
+ }
return "redirect:/apikeys/mykeys";
} |
||||||||||||||||||||||||||||||||||||||||||||||||||||
| return "redirect:/apikeys/mykeys"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| @PostMapping("mykeys/nameupdate") | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| public String nameUpdate(@RequestParam String apiKeyName, @RequestParam String referenceId, @AuthenticationPrincipal OidcUser user) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| if(issuedApiKeyService.isValidUuid(UUID.fromString(referenceId), user.getName())) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| var update = new UpdateApiKey(UUID.fromString(referenceId), apiKeyName, false, null); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| issuedApiKeyService.updateApiKey(update); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| return "redirect:/apikeys/mykeys"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+106
to
+111
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Add error handling for nameUpdate method Similar to the revokeKey method, this method needs proper error handling for UUID validation and user feedback. @PostMapping("mykeys/nameupdate")
- public String nameUpdate(@RequestParam String apiKeyName, @RequestParam String referenceId, @AuthenticationPrincipal OidcUser user) {
- if(issuedApiKeyService.isValidUuid(UUID.fromString(referenceId), user.getName())) {
- var update = new UpdateApiKey(UUID.fromString(referenceId), apiKeyName, false, null);
- issuedApiKeyService.updateApiKey(update);
- }
+ public String nameUpdate(@RequestParam String apiKeyName, @RequestParam String referenceId,
+ @AuthenticationPrincipal OidcUser user, RedirectAttributes redirectAttributes) {
+ try {
+ UUID uuid = UUID.fromString(referenceId);
+ if (issuedApiKeyService.isValidUuid(uuid, user.getName())) {
+ var update = new UpdateApiKey(uuid, apiKeyName, false, null);
+ issuedApiKeyService.updateApiKey(update);
+ redirectAttributes.addFlashAttribute("success", "API key name updated successfully");
+ } else {
+ redirectAttributes.addFlashAttribute("error", "You don't have permission to update this API key");
+ }
+ } catch (IllegalArgumentException e) {
+ redirectAttributes.addFlashAttribute("error", "Invalid API key reference");
+ }
return "redirect:/apikeys/mykeys";
}📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,94 @@ | ||
| package org.storkforge.barkr.config; | ||
|
|
||
| import org.springframework.context.annotation.Bean; | ||
| import org.springframework.context.annotation.Configuration; | ||
| import org.springframework.core.annotation.Order; | ||
| import org.springframework.http.HttpMethod; | ||
| import org.springframework.security.config.Customizer; | ||
| import org.springframework.security.config.annotation.method.configuration.EnableMethodSecurity; | ||
| 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.AbstractHttpConfigurer; | ||
| import org.springframework.security.config.http.SessionCreationPolicy; | ||
| import org.springframework.security.web.SecurityFilterChain; | ||
| import org.springframework.security.web.authentication.logout.LogoutFilter; | ||
| import org.storkforge.barkr.domain.IssuedApiKeyService; | ||
| import org.storkforge.barkr.domain.roles.BarkrRole; | ||
| import org.storkforge.barkr.filters.ApiKeyAuthenticationFilter; | ||
|
|
||
| @Configuration | ||
| @EnableWebSecurity | ||
| @EnableMethodSecurity | ||
| public class SecurityConfig { | ||
|
|
||
| @Bean | ||
| @Order(3) | ||
| public SecurityFilterChain securityFilterChain(HttpSecurity http) throws Exception{ | ||
| http | ||
| .oauth2Login(Customizer.withDefaults()) | ||
| .authorizeHttpRequests(authorize -> authorize | ||
| .requestMatchers("/","/login", "/error", "/css/**", "/js/**", "/images/**").permitAll() | ||
| .requestMatchers("/post/load").permitAll() | ||
| .requestMatchers( "/account/{id}/image").permitAll() | ||
| .requestMatchers("/ai/generate").hasRole(BarkrRole.PREMIUM.name()) | ||
| .requestMatchers("/post/add").authenticated() | ||
| .requestMatchers("/account/{id}/upload").authenticated() | ||
| .requestMatchers("/{username}").authenticated() | ||
| .requestMatchers("/apikeys/generate").authenticated() | ||
| .requestMatchers("/apikeys/apikeyform").authenticated() | ||
| .requestMatchers("/apikeys/result").authenticated() | ||
| .requestMatchers("/apikeys/mykeys").authenticated() | ||
| .requestMatchers("/apikeys/mykeys/revoke").authenticated() | ||
| .requestMatchers("/apikeys/mykeys/nameupdate").authenticated() | ||
| .requestMatchers("/unlock-easter-egg").authenticated() | ||
|
|
||
| .anyRequest().denyAll() | ||
|
|
||
| ) | ||
| .logout(logout -> logout | ||
| .logoutUrl("/logout") | ||
| .invalidateHttpSession(true) | ||
| .clearAuthentication(true) | ||
| .deleteCookies("JSESSIONID") | ||
| .logoutSuccessUrl("/barkr/logout") | ||
| .permitAll() | ||
|
|
||
| ).csrf(csrf -> csrf | ||
| .ignoringRequestMatchers("/logout", "/ai/generate") | ||
| ); | ||
|
|
||
| return http.build(); | ||
| } | ||
|
|
||
|
|
||
| @Bean | ||
| @Order(2) | ||
| public SecurityFilterChain restAPIAndGraphQLFilterChain( | ||
| HttpSecurity http, | ||
| IssuedApiKeyService issuedApiKeyService) throws Exception { | ||
| http.securityMatcher("/api/**", "/graphql") | ||
| .sessionManagement(session -> session.sessionCreationPolicy(SessionCreationPolicy.STATELESS)) | ||
| .csrf(AbstractHttpConfigurer::disable) | ||
| .addFilterAfter(new ApiKeyAuthenticationFilter(issuedApiKeyService), LogoutFilter.class) | ||
| .authorizeHttpRequests( | ||
| authorize -> authorize | ||
| .requestMatchers("/api/accounts").authenticated() | ||
| .requestMatchers("/api/posts").authenticated() | ||
| .requestMatchers("/api/accounts/{id}").authenticated() | ||
| .requestMatchers("/api/posts/{id}").authenticated() | ||
| .requestMatchers("/api/keys").authenticated() | ||
| .requestMatchers(HttpMethod.POST, "/graphql").authenticated() | ||
| .anyRequest().denyAll() | ||
|
|
||
| ); | ||
| return http.build(); | ||
|
|
||
| } | ||
|
|
||
|
|
||
|
|
||
|
|
||
|
|
||
|
|
||
|
|
||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add null check for user and optional handling
The current implementation doesn't handle the case where the user might not be found. Add proper null checks and Optional handling.
@GetMapping("/mykeys") public String myKeys(Model model , @AuthenticationPrincipal OidcUser user) { - var currentUser = issuedApiKeyService.findByGoogleOidc2Id(user.getName()); - var keys = issuedApiKeyService.allApiKeys(user.getName()); - model.addAttribute("account", currentUser.get().getAccount()); + var currentUser = issuedApiKeyService.findByGoogleOidc2Id(user.getName()); + if (currentUser.isEmpty()) { + return "redirect:/error"; + } + var keys = issuedApiKeyService.allApiKeys(user.getName()); + model.addAttribute("account", currentUser.get().getAccount()); model.addAttribute("keys", keys.apiKeys()); return "apikeys/mykeys";📝 Committable suggestion