Skip to content
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

spring-boot: fix sonar security issue #28729

Merged
merged 1 commit into from
Feb 19, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.http.HttpStatus;
import org.springframework.http.MediaType;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.*;
<%_ if (reactive) { _%>
import java.util.Objects;
Expand Down Expand Up @@ -135,15 +136,15 @@ public class AccountResource {

<%_ if (!authenticationTypeJwt) { _%>
/**
* {@code GET /authenticate} : check if the user is authenticated, and return its login.
* {@code GET /authenticate} : check if the user is authenticated.
*
* @param principal the authentication principal.
* @return the login if the user is authenticated.
* @return the {@link ResponseEntity} with status {@code 204 (No Content)},
* or with status {@code 401 (Unauthorized)} if not authenticated.
*/
@GetMapping(value = "/authenticate", produces = MediaType.TEXT_PLAIN_VALUE)
public String isAuthenticated(Principal principal) {
@GetMapping("/authenticate")
public ResponseEntity<Void> isAuthenticated(Principal principal) {
LOG.debug("REST request to check if the current user is authenticated");
return principal == null ? null : principal.getName();
return ResponseEntity.status(principal == null ? HttpStatus.UNAUTHORIZED : HttpStatus.NO_CONTENT).build();
}

<%_ } _%>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@ import <%= packageName %>.service.dto.<%= user.adminUserDto %>;
import java.security.Principal;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.http.HttpStatus;
import org.springframework.http.MediaType;
import org.springframework.http.ResponseEntity;
import org.springframework.security.authentication.AbstractAuthenticationToken;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.RequestMapping;
Expand Down Expand Up @@ -74,14 +76,14 @@ public class AccountResource {
}

/**
* {@code GET /authenticate} : check if the user is authenticated, and return its login.
* {@code GET /authenticate} : check if the user is authenticated.
*
* @param principal the authentication principal.
* @return the login if the user is authenticated.
* @return the {@link ResponseEntity} with status {@code 204 (No Content)},
* or with status {@code 401 (Unauthorized)} if not authenticated.
*/
@GetMapping(value = "/authenticate", produces = MediaType.TEXT_PLAIN_VALUE)
public String isAuthenticated(Principal principal) {
@GetMapping("/authenticate")
public ResponseEntity<Void> isAuthenticated(Principal principal) {
LOG.debug("REST request to check if the current user is authenticated");
return principal == null ? null : principal.getName();
return ResponseEntity.status(principal == null ? HttpStatus.UNAUTHORIZED : HttpStatus.NO_CONTENT).build();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,9 @@ import org.springframework.security.core.context.SecurityContextHolder;
import com.fasterxml.jackson.annotation.JsonAnyGetter;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.http.HttpStatus;
import org.springframework.http.MediaType;
import org.springframework.http.ResponseEntity;
import org.springframework.security.core.GrantedAuthority;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.RequestMapping;
Expand Down Expand Up @@ -139,15 +141,15 @@ public class AccountResource {
<%_ if (!authenticationTypeJwt) { _%>

/**
* {@code GET /authenticate} : check if the user is authenticated, and return its login.
* {@code GET /authenticate} : check if the user is authenticated.
*
* @param principal the authentication principal.
* @return the login if the user is authenticated.
* @return the {@link ResponseEntity} with status {@code 204 (No Content)},
* or with status {@code 401 (Unauthorized)} if not authenticated.
*/
@GetMapping(value = "/authenticate", produces = MediaType.TEXT_PLAIN_VALUE)
public String isAuthenticated(Principal principal) {
@GetMapping("/authenticate")
public ResponseEntity<Void> isAuthenticated(Principal principal) {
LOG.debug("REST request to check if the current user is authenticated");
return principal == null ? null : principal.getName();
return ResponseEntity.status(principal == null ? HttpStatus.UNAUTHORIZED : HttpStatus.NO_CONTENT).build();
}
<%_ } _%>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,15 +117,15 @@ public class AuthenticateController {
<%_ } _%>

/**
* {@code GET /authenticate} : check if the user is authenticated, and return its login.
* {@code GET /authenticate} : check if the user is authenticated.
*
* @param principal the authentication principal.
* @return the login if the user is authenticated.
* @return the {@link ResponseEntity} with status {@code 204 (No Content)},
* or with status {@code 401 (Unauthorized)} if not authenticated.
*/
@GetMapping(value = "/authenticate", produces = MediaType.TEXT_PLAIN_VALUE)
public String isAuthenticated(Principal principal) {
@GetMapping("/authenticate")
public ResponseEntity<Void> isAuthenticated(Principal principal) {
LOG.debug("REST request to check if the current user is authenticated");
return principal == null ? null : principal.getName();
return ResponseEntity.status(principal == null ? HttpStatus.UNAUTHORIZED : HttpStatus.NO_CONTENT).build();
}

public String createToken(Authentication authentication, boolean rememberMe) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package <%= packageName %>.security.jwt;

import org.springframework.http.HttpStatus;
import org.springframework.http.MediaType;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.*;

/**
Expand All @@ -12,11 +14,9 @@ public class TestAuthenticationResource {

/**
* {@code GET /authenticate} : check if the authentication token correctly validates
*
* @return ok.
*/
@GetMapping(value = "/authenticate", produces = MediaType.TEXT_PLAIN_VALUE)
public String isAuthenticated() {
return "ok";
@GetMapping("/authenticate")
public ResponseEntity<Void> isAuthenticated() {
return ResponseEntity.status(HttpStatus.NO_CONTENT).build();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ class TokenAuthenticationIT {
<%_ if (reactive) { %>

private void expectOk(String token) {
webTestClient.get().uri("/api/authenticate").headers(headers -> headers.setBearerAuth(token)).exchange().expectStatus().isOk();
webTestClient.get().uri("/api/authenticate").headers(headers -> headers.setBearerAuth(token)).exchange().expectStatus().isNoContent();
}

private void expectUnauthorized(String token) {
Expand All @@ -72,7 +72,7 @@ class TokenAuthenticationIT {
<%_ } else { _%>

private void expectOk(String token) throws Exception {
mvc.perform(MockMvcRequestBuilders.get("/api/authenticate").header(AUTHORIZATION, BEARER + token)).andExpect(status().isOk());
mvc.perform(MockMvcRequestBuilders.get("/api/authenticate").header(AUTHORIZATION, BEARER + token)).andExpect(status().isNoContent());
}

private void expectUnauthorized(String token) throws Exception {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,10 +97,7 @@ class TokenAuthenticationSecurityMetersIT {
.get()
.uri("/api/authenticate")
.headers(headers -> headers.setBearerAuth(token))
.exchange()
.returnResult(String.class)
.getResponseBody()
.blockLast();
.exchange();
}
<%_ } else { _%>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ class SpaWebFilterIT {
@Test
void testFilterDoesNotForwardToIndexForApi() throws Exception {
mockMvc.perform(get("/api/<% if (!applicationTypeMicroservice) { %>authenticate<% } else { %>users<% } %>"))
.andExpect(status().isOk())
.andExpect(status().is2xxSuccessful())
.andExpect(forwardedUrl(null));
}
<%_ } _%>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,7 @@ class SpaWebFilterIT {
.get()
.uri("/api/<% if (!applicationTypeMicroservice) { %>authenticate<% } else { %>users<% } %>")
.exchange()
.expectStatus().isOk()
.expectBody(String.class).isEqualTo("user");
.expectStatus().is2xxSuccessful();
}
<%_ } _%>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,16 +174,13 @@ class AccountResourceIT {
<%_ if (reactive) { _%>
void testNonAuthenticatedUser() {
accountWebTestClient.get().uri("/api/authenticate")
.accept(MediaType.TEXT_PLAIN)
.exchange()
.expectStatus().isOk()
.expectBody().isEmpty();
.expectStatus().isUnauthorized();
<%_ } else { _%>
void testNonAuthenticatedUser() throws Exception {
restAccountMockMvc
.perform(get("/api/authenticate").accept(MediaType.TEXT_PLAIN))
.andExpect(status().isOk())
.andExpect(content().string(""));
.perform(get("/api/authenticate"))
.andExpect(status().isUnauthorized());
<%_ } _%>
}

Expand All @@ -193,16 +190,13 @@ class AccountResourceIT {
void testAuthenticatedUser() {
accountWebTestClient
.get().uri("/api/authenticate")
.accept(MediaType.TEXT_PLAIN)
.exchange()
.expectStatus().isOk()
.expectBody(String.class).isEqualTo(TEST_USER_LOGIN);
.expectStatus().isNoContent();
<%_ } else { _%>
void testAuthenticatedUser() throws Exception {
restAccountMockMvc
.perform(get("/api/authenticate").with(request -> request).accept(MediaType.TEXT_PLAIN))
.andExpect(status().isOk())
.andExpect(content().string(TEST_USER_LOGIN));
.perform(get("/api/authenticate").with(request -> request))
.andExpect(status().isNoContent());
<%_ } _%>
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -185,15 +185,11 @@ class AccountResourceIT {
void testNonAuthenticatedUser() <% if (!reactive) { %>throws Exception <% } %>{
<%_ if (reactive) { _%>
webTestClient.get().uri("/api/authenticate")
.accept(MediaType.TEXT_PLAIN)
.exchange()
.expectStatus().isOk()
.expectBody().isEmpty();
.expectStatus().isUnauthorized();
<%_ } else { _%>
restAccountMockMvc.perform(get("/api/authenticate")
.accept(MediaType.TEXT_PLAIN))
.andExpect(status().isOk())
.andExpect(content().string(""));
restAccountMockMvc.perform(get("/api/authenticate"))
.andExpect(status().isUnauthorized());
<%_ } _%>
}

Expand All @@ -203,16 +199,11 @@ class AccountResourceIT {
<%_ if (reactive) { _%>
webTestClient
.get().uri("/api/authenticate")
.accept(MediaType.TEXT_PLAIN)
.exchange()
.expectStatus().isOk()
.expectBody(String.class).isEqualTo(TEST_USER_LOGIN);
.expectStatus().isNoContent();
<%_ } else { _%>
restAccountMockMvc.perform(get("/api/authenticate")
.with(request -> request)
.accept(MediaType.TEXT_PLAIN))
.andExpect(status().isOk())
.andExpect(content().string(TEST_USER_LOGIN));
restAccountMockMvc.perform(get("/api/authenticate").with(request -> request))
.andExpect(status().isNoContent());
<%_ } _%>
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -116,15 +116,11 @@ class AccountResourceIT {
void testNonAuthenticatedUser() <% if (!reactive) { %>throws Exception <% } %>{
<%_ if (reactive) { _%>
accountWebTestClient.get().uri("/api/authenticate")
.accept(MediaType.TEXT_PLAIN)
.exchange()
.expectStatus().isOk()
.expectBody().isEmpty();
.expectStatus().isUnauthorized();
<%_ } else { _%>
restAccountMockMvc.perform(get("/api/authenticate")
.accept(MediaType.TEXT_PLAIN))
.andExpect(status().isOk())
.andExpect(content().string(""));
restAccountMockMvc.perform(get("/api/authenticate"))
.andExpect(status().isUnauthorized());
<%_ } _%>
}

Expand All @@ -134,16 +130,11 @@ class AccountResourceIT {
<%_ if (reactive) { _%>
accountWebTestClient
.get().uri("/api/authenticate")
.accept(MediaType.TEXT_PLAIN)
.exchange()
.expectStatus().isOk()
.expectBody(String.class).isEqualTo(TEST_USER_LOGIN);
.expectStatus().isNoContent();
<%_ } else { _%>
restAccountMockMvc.perform(get("/api/authenticate")
.with(request -> request)
.accept(MediaType.TEXT_PLAIN))
.andExpect(status().isOk())
.andExpect(content().string(TEST_USER_LOGIN));
restAccountMockMvc.perform(get("/api/authenticate").with(request -> request))
.andExpect(status().isNoContent());
<%_ } _%>
}
}
Loading