Commit 0a1ff491 authored by Matija Obreza's avatar Matija Obreza
Browse files

Updated OAuth service

- Allow for blank secret if client doesn't use client_credentials grant
- Updating the Client maintains original secret
- Client#roles converted to a Set
parent 7c42917c
......@@ -15,7 +15,6 @@
*/
package org.genesys.blocks.oauth.model;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.HashSet;
......@@ -38,9 +37,6 @@ import javax.persistence.PreUpdate;
import javax.persistence.Table;
import javax.persistence.Transient;
import com.fasterxml.jackson.annotation.JsonIgnore;
import com.fasterxml.jackson.annotation.JsonView;
import org.apache.commons.lang3.StringUtils;
import org.genesys.blocks.model.Copyable;
import org.genesys.blocks.model.JsonViews;
......@@ -48,6 +44,9 @@ import org.genesys.blocks.security.model.AclSid;
import org.springframework.security.core.GrantedAuthority;
import org.springframework.security.oauth2.provider.ClientDetails;
import com.fasterxml.jackson.annotation.JsonIgnore;
import com.fasterxml.jackson.annotation.JsonView;
/**
* OAuth Client information.
*
......@@ -131,7 +130,7 @@ public class OAuthClient extends AclSid implements ClientDetails, Copyable<OAuth
@Enumerated(EnumType.STRING)
@CollectionTable(name = "oauthclientrole", joinColumns = @JoinColumn(name = "clientId"))
@Column(name = "oauthclientrole")
private Collection<OAuthRole> roles = new ArrayList<>();
private Set<OAuthRole> roles = new HashSet<>();
/** The additional information. */
@Transient
......@@ -302,7 +301,7 @@ public class OAuthClient extends AclSid implements ClientDetails, Copyable<OAuth
*
* @param roles the new roles
*/
public void setRoles(final Collection<OAuthRole> roles) {
public void setRoles(final Set<OAuthRole> roles) {
this.roles = roles;
}
......@@ -559,7 +558,10 @@ public class OAuthClient extends AclSid implements ClientDetails, Copyable<OAuth
@Override
public OAuthClient apply(OAuthClient source) {
String oldSecret = this.clientSecret;
Copyable.super.apply(source);
// Keep old secret
this.clientSecret = oldSecret;
this.autoApproveScopes.clear();
this.autoApproveScopes.addAll(source.autoApproveScopes);
......
/*
* Copyright 2017 Global Crop Diversity Trust
* Copyright 2018 Global Crop Diversity Trust
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
......@@ -90,11 +90,19 @@ public interface OAuthClientDetailsService extends ClientDetailsService {
List<OAuthClient> autocompleteClients(String title, int limit);
/**
* Generates a new clientSecret
*
* Generates a new clientSecret.
*
* @param oauthClient the client to regenerate secret for
* @return the new cliet secret
*/
String resetSecret(OAuthClient oauthClient);
/**
* Removes the Client secret.
*
* @param oauthClient the oauth client
* @return Updated OAuth client
*/
OAuthClient removeSecret(OAuthClient oauthClient);
}
......@@ -713,4 +713,17 @@ public class OAuthServiceImpl implements OAuthClientDetailsService, OAuthTokenSt
oauthClient = oauthClientRepository.save(oauthClient);
return clientSecret;
}
@Override
@Transactional
@PreAuthorize("hasRole('ADMINISTRATOR') or hasPermission(#oauthClient, 'ADMINISTRATION')")
public final OAuthClient removeSecret(OAuthClient oauthClient) {
oauthClient = oauthClientRepository.findOne(oauthClient.getId());
if (oauthClient.getAuthorizedGrantTypes().contains("client_credentials")) {
throw new RuntimeException("OAuth Client with client_credentials grant must have a secret");
}
oauthClient.setClientSecret(null);
oauthClient = oauthClientRepository.save(oauthClient);
return lazyLoad(oauthClient);
}
}
......@@ -52,13 +52,15 @@ public class OAuthClientTest extends AbstractRestTest {
@Test
public void updateClient() {
OAuthClient client = oauthClientDetailsService.addClient(makeClient());
String originalSecret = client.getClientSecret();
client.setTitle("AAAABBBBCCC");
client.setClientSecret("Some secret");
OAuthClient updatedClient = oauthClientDetailsService.updateClient(client.getId(), client.getVersion(), client);
assertThat("OAuthClient#title must be updated", updatedClient.getTitle(), is("AAAABBBBCCC"));
assertThat("OAuthClient#clientId must not be updated", updatedClient.getClientId(), is(client.getClientId()));
assertThat("OAuthClient#clientSecret must not be updated", updatedClient.getClientSecret(), is(client.getClientSecret()));
assertThat("OAuthClient#clientSecret must not be updated", updatedClient.getClientSecret(), is(originalSecret));
assertThat("Autocomplete must return the 1 client", oauthClientDetailsService.autocompleteClients(updatedClient.getTitle().substring(0, 10), 10), hasSize(1));
assertThat("Autocomplete must return the 1 client", oauthClientDetailsService.autocompleteClients(updatedClient.getTitle().substring(0, 5), 10), hasSize(1));
......@@ -68,10 +70,20 @@ public class OAuthClientTest extends AbstractRestTest {
@Test
public void resetSecret() {
OAuthClient client = oauthClientDetailsService.addClient(makeClient());
assertThat("OAuthClient#clientSecret must be generated", client.getClientSecret(), not(nullValue()));
String oldSecret = client.getClientSecret();
String newSecret = oauthClientDetailsService.resetSecret(client);
assertThat("OAuthClient#clientSecret must not be updated", newSecret, not(is(oldSecret)));
assertThat("OAuthClient#clientSecret must be updated", newSecret, not(is(oldSecret)));
}
@Test
public void clearSecret() {
OAuthClient client = oauthClientDetailsService.addClient(makeClient());
client = oauthClientDetailsService.removeSecret(client);
assertThat("OAuthClient#clientSecret must be null", client.getClientSecret(), nullValue());
}
private OAuthClient makeClient() {
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment