Polish JdbcOAuth2AuthorizationConsentService

Issue gh-314

oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/JdbcOAuth2AuthorizationConsentService.java

@@ -40,7 +40,6 @@ import org.springframework.util.StringUtils;
 
 /**
  * A JDBC implementation of an {@link OAuth2AuthorizationConsentService} that uses a
- * <p>
  * {@link JdbcOperations} for {@link OAuth2AuthorizationConsent} persistence.
  *
  * <p>
@@ -50,11 +49,11 @@ import org.springframework.util.StringUtils;
  * therefore MUST be defined in the database schema.
  *
  * @author Ovidiu Popa
+ * @since 0.1.2
  * @see OAuth2AuthorizationConsentService
  * @see OAuth2AuthorizationConsent
  * @see JdbcOperations
  * @see RowMapper
- * @since 0.1.2
  */
 public class JdbcOAuth2AuthorizationConsentService implements OAuth2AuthorizationConsentService {
 
@@ -109,10 +108,8 @@ public class JdbcOAuth2AuthorizationConsentService implements OAuth2Authorizatio
 	@Override
 	public void save(OAuth2AuthorizationConsent authorizationConsent) {
 		Assert.notNull(authorizationConsent, "authorizationConsent cannot be null");
-
-		OAuth2AuthorizationConsent existingAuthorizationConsent =
-				findById(authorizationConsent.getRegisteredClientId(), authorizationConsent.getPrincipalName());
-
+		OAuth2AuthorizationConsent existingAuthorizationConsent = findById(
+				authorizationConsent.getRegisteredClientId(), authorizationConsent.getPrincipalName());
 		if (existingAuthorizationConsent == null) {
 			insertAuthorizationConsent(authorizationConsent);
 		} else {
@@ -205,7 +202,6 @@ public class JdbcOAuth2AuthorizationConsentService implements OAuth2Authorizatio
 	 * {@code ResultSet} to {@link OAuth2AuthorizationConsent}.
 	 */
 	public static class OAuth2AuthorizationConsentRowMapper implements RowMapper<OAuth2AuthorizationConsent> {
-
 		private final RegisteredClientRepository registeredClientRepository;
 
 		public OAuth2AuthorizationConsentRowMapper(RegisteredClientRepository registeredClientRepository) {
@@ -216,9 +212,7 @@ public class JdbcOAuth2AuthorizationConsentService implements OAuth2Authorizatio
 		@Override
 		public OAuth2AuthorizationConsent mapRow(ResultSet rs, int rowNum) throws SQLException {
 			String registeredClientId = rs.getString("registered_client_id");
-
-			RegisteredClient registeredClient = this.registeredClientRepository
-					.findById(registeredClientId);
+			RegisteredClient registeredClient = this.registeredClientRepository.findById(registeredClientId);
 			if (registeredClient == null) {
 				throw new DataRetrievalFailureException(
 						"The RegisteredClient with id '" + registeredClientId + "' was not found in the RegisteredClientRepository.");

oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/JdbcOAuth2AuthorizationConsentServiceTests.java

@@ -56,7 +56,6 @@ import static org.mockito.Mockito.when;
  * @author Ovidiu Popa
  */
 public class JdbcOAuth2AuthorizationConsentServiceTests {
-
 	private static final String OAUTH2_AUTHORIZATION_CONSENT_SCHEMA_SQL_RESOURCE = "org/springframework/security/oauth2/server/authorization/oauth2-authorization-consent-schema.sql";
 	private static final String CUSTOM_OAUTH2_AUTHORIZATION_CONSENT_SCHEMA_SQL_RESOURCE = "org/springframework/security/oauth2/server/authorization/custom-oauth2-authorization-consent-schema.sql";
 	private static final String PRINCIPAL_NAME = "principal-name";
@@ -64,7 +63,11 @@ public class JdbcOAuth2AuthorizationConsentServiceTests {
 
 	private static final OAuth2AuthorizationConsent AUTHORIZATION_CONSENT =
 			OAuth2AuthorizationConsent.withId(REGISTERED_CLIENT.getId(), PRINCIPAL_NAME)
-					.authority(new SimpleGrantedAuthority("some.authority"))
+					.authority(new SimpleGrantedAuthority("SCOPE_scope1"))
+					.authority(new SimpleGrantedAuthority("SCOPE_scope2"))
+					.authority(new SimpleGrantedAuthority("SCOPE_scope3"))
+					.authority(new SimpleGrantedAuthority("authority-a"))
+					.authority(new SimpleGrantedAuthority("authority-b"))
 					.build();
 
 	private EmbeddedDatabase db;
@@ -72,6 +75,19 @@ public class JdbcOAuth2AuthorizationConsentServiceTests {
 	private RegisteredClientRepository registeredClientRepository;
 	private JdbcOAuth2AuthorizationConsentService authorizationConsentService;
 
+	@Before
+	public void setUp() {
+		this.db = createDb();
+		this.jdbcOperations = new JdbcTemplate(this.db);
+		this.registeredClientRepository = mock(RegisteredClientRepository.class);
+		this.authorizationConsentService = new JdbcOAuth2AuthorizationConsentService(this.jdbcOperations, this.registeredClientRepository);
+	}
+
+	@After
+	public void tearDown() {
+		this.db.shutdown();
+	}
+
 	@Test
 	public void constructorWhenJdbcOperationsIsNullThenThrowIllegalArgumentException() {
 		// @formatter:off
@@ -127,7 +143,7 @@ public class JdbcOAuth2AuthorizationConsentServiceTests {
 		RegisteredClient newRegisteredClient = TestRegisteredClients.registeredClient()
 				.id("new-client").build();
 
-		when(registeredClientRepository.findById(eq(newRegisteredClient.getId())))
+		when(this.registeredClientRepository.findById(eq(newRegisteredClient.getId())))
 				.thenReturn(newRegisteredClient);
 
 		this.authorizationConsentService.save(expectedAuthorizationConsent);
@@ -143,7 +159,7 @@ public class JdbcOAuth2AuthorizationConsentServiceTests {
 				OAuth2AuthorizationConsent.from(AUTHORIZATION_CONSENT)
 						.authority(new SimpleGrantedAuthority("new.authority"))
 						.build();
-		when(registeredClientRepository.findById(eq(REGISTERED_CLIENT.getId())))
+		when(this.registeredClientRepository.findById(eq(REGISTERED_CLIENT.getId())))
 				.thenReturn(REGISTERED_CLIENT);
 
 		this.authorizationConsentService.save(expectedAuthorizationConsent);
@@ -157,7 +173,7 @@ public class JdbcOAuth2AuthorizationConsentServiceTests {
 
 	@Test
 	public void saveLoadAuthorizationConsentWhenCustomStrategiesSetThenCalled() throws Exception {
-		when(registeredClientRepository.findById(eq(REGISTERED_CLIENT.getId())))
+		when(this.registeredClientRepository.findById(eq(REGISTERED_CLIENT.getId())))
 				.thenReturn(REGISTERED_CLIENT);
 
 		JdbcOAuth2AuthorizationConsentService.OAuth2AuthorizationConsentRowMapper authorizationConsentRowMapper = spy(
@@ -205,6 +221,17 @@ public class JdbcOAuth2AuthorizationConsentServiceTests {
 				.withMessage("principalName cannot be empty");
 	}
 
+	@Test
+	public void findByIdWhenAuthorizationConsentExistsThenFound() {
+		when(this.registeredClientRepository.findById(eq(REGISTERED_CLIENT.getId())))
+				.thenReturn(REGISTERED_CLIENT);
+
+		this.authorizationConsentService.save(AUTHORIZATION_CONSENT);
+		OAuth2AuthorizationConsent authorizationConsent = this.authorizationConsentService.findById(
+				AUTHORIZATION_CONSENT.getRegisteredClientId(), AUTHORIZATION_CONSENT.getPrincipalName());
+		assertThat(authorizationConsent).isNotNull();
+	}
+
 	@Test
 	public void findByIdWhenAuthorizationConsentDoesNotExistThenNull() {
 		this.authorizationConsentService.save(AUTHORIZATION_CONSENT);
@@ -221,27 +248,16 @@ public class JdbcOAuth2AuthorizationConsentServiceTests {
 		OAuth2AuthorizationConsentService authorizationConsentService =
 				new CustomJdbcOAuth2AuthorizationConsentService(new JdbcTemplate(db), this.registeredClientRepository);
 		authorizationConsentService.save(AUTHORIZATION_CONSENT);
-		OAuth2AuthorizationConsent foundAuthorizationConsent1 = authorizationConsentService.findById(AUTHORIZATION_CONSENT.getRegisteredClientId(), AUTHORIZATION_CONSENT.getPrincipalName());
+		OAuth2AuthorizationConsent foundAuthorizationConsent1 = authorizationConsentService.findById(
+				AUTHORIZATION_CONSENT.getRegisteredClientId(), AUTHORIZATION_CONSENT.getPrincipalName());
 		assertThat(foundAuthorizationConsent1).isEqualTo(AUTHORIZATION_CONSENT);
 		authorizationConsentService.remove(AUTHORIZATION_CONSENT);
-		OAuth2AuthorizationConsent foundAuthorizationConsent2 = authorizationConsentService.findById(REGISTERED_CLIENT.getClientId(), AUTHORIZATION_CONSENT.getPrincipalName());
+		OAuth2AuthorizationConsent foundAuthorizationConsent2 = authorizationConsentService.findById(
+				AUTHORIZATION_CONSENT.getRegisteredClientId(), AUTHORIZATION_CONSENT.getPrincipalName());
 		assertThat(foundAuthorizationConsent2).isNull();
 		db.shutdown();
 	}
 
-	@Before
-	public void setUp() {
-		this.db = createDb();
-		this.registeredClientRepository = mock(RegisteredClientRepository.class);
-		this.jdbcOperations = new JdbcTemplate(this.db);
-		this.authorizationConsentService = new JdbcOAuth2AuthorizationConsentService(this.jdbcOperations, this.registeredClientRepository);
-	}
-
-	@After
-	public void tearDown() {
-		this.db.shutdown();
-	}
-
 	private static EmbeddedDatabase createDb() {
 		return createDb(OAUTH2_AUTHORIZATION_CONSENT_SCHEMA_SQL_RESOURCE);
 	}
@@ -282,7 +298,7 @@ public class JdbcOAuth2AuthorizationConsentServiceTests {
 
 		private static final String REMOVE_AUTHORIZATION_CONSENT_SQL = "DELETE FROM " + TABLE_NAME + " WHERE " + PK_FILTER;
 
-		CustomJdbcOAuth2AuthorizationConsentService(JdbcOperations jdbcOperations, RegisteredClientRepository registeredClientRepository) {
+		private CustomJdbcOAuth2AuthorizationConsentService(JdbcOperations jdbcOperations, RegisteredClientRepository registeredClientRepository) {
 			super(jdbcOperations, registeredClientRepository);
 			setAuthorizationConsentRowMapper(new CustomOAuth2AuthorizationConsentRowMapper(registeredClientRepository));
 		}
@@ -317,16 +333,14 @@ public class JdbcOAuth2AuthorizationConsentServiceTests {
 
 		private static final class CustomOAuth2AuthorizationConsentRowMapper extends JdbcOAuth2AuthorizationConsentService.OAuth2AuthorizationConsentRowMapper {
 
-			CustomOAuth2AuthorizationConsentRowMapper(RegisteredClientRepository registeredClientRepository) {
+			private CustomOAuth2AuthorizationConsentRowMapper(RegisteredClientRepository registeredClientRepository) {
 				super(registeredClientRepository);
 			}
 
 			@Override
 			public OAuth2AuthorizationConsent mapRow(ResultSet rs, int rowNum) throws SQLException {
 				String registeredClientId = rs.getString("registeredClientId");
-
-				RegisteredClient registeredClient = getRegisteredClientRepository()
-						.findById(registeredClientId);
+				RegisteredClient registeredClient = getRegisteredClientRepository().findById(registeredClientId);
 				if (registeredClient == null) {
 					throw new DataRetrievalFailureException(
 							"The RegisteredClient with id '" + registeredClientId + "' was not found in the RegisteredClientRepository.");