Skip to content

Commit 0f799ec

Browse files
pfuriopfurio
committed
catalog: fix AuthenticationManager implementation, #TASK-7192
1 parent 727b48e commit 0f799ec

File tree

8 files changed

+27
-169
lines changed

8 files changed

+27
-169
lines changed

‎opencga-catalog/src/main/java/org/opencb/opencga/catalog/auth/authentication/AuthenticationManager.java

Lines changed: 6 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -79,20 +79,6 @@ Key converStringToKeyObject(String keyString, String jcaAlgorithm) {
7979
public abstract AuthenticationResponse authenticate(String organizationId, String userId, String password)
8080
throws CatalogAuthenticationException;
8181

82-
/**
83-
* Authenticate the user against the Authentication server.
84-
*
85-
* @param organizationId Organization id.
86-
* @param userId User to authenticate
87-
* @param password Password.
88-
* @param secretKey Secret key to apply to the token.
89-
* @return AuthenticationResponse object.
90-
* @throws CatalogAuthenticationException CatalogAuthenticationException if any of the credentials are wrong or the access is denied
91-
* for any other reason.
92-
*/
93-
public abstract AuthenticationResponse authenticate(String organizationId, String userId, String password, String secretKey)
94-
throws CatalogAuthenticationException;
95-
9682
/**
9783
* Authenticate the user against the Authentication server.
9884
*
@@ -114,19 +100,15 @@ public AuthenticationResponse refreshToken(String refreshToken) throws CatalogAu
114100
* Validates that the token is valid.
115101
*
116102
* @param token token that have been assigned to a user.
117-
* @param secretKey secret key to be used for the token validation (may be null).
103+
* @param securityKey key used to fully ensure it corresponds to that user.
118104
* @throws CatalogAuthenticationException when the token does not correspond to any user, is expired or has been altered.
119105
*/
120-
public void validateToken(String token, @Nullable String secretKey) throws CatalogAuthenticationException {
106+
public void validateToken(String token, @Nullable String securityKey) throws CatalogAuthenticationException {
121107
if (StringUtils.isEmpty(token) || "null".equalsIgnoreCase(token)) {
122108
throw new CatalogAuthenticationException("Token is null or empty.");
123109
}
124-
Key privateKey = null;
125-
if (secretKey != null) {
126-
privateKey = converStringToKeyObject(secretKey, jwtManager.getAlgorithm().getJcaName());
127-
}
128110

129-
jwtManager.validateToken(token, privateKey);
111+
jwtManager.validateToken(token);
130112
}
131113

132114
/**
@@ -208,20 +190,7 @@ public abstract void changePassword(String organizationId, String userId, String
208190
* @return A token.
209191
*/
210192
public String createToken(String organizationId, String userId) throws CatalogAuthenticationException {
211-
return createToken(organizationId, userId, Collections.emptyMap(), expiration, (Key) null);
212-
}
213-
214-
/**
215-
* Create a token for the user with default expiration time.
216-
*
217-
* @param organizationId Organization id.
218-
* @param userId user.
219-
* @param secretKey secret key to be used for the token generation.
220-
* @throws CatalogAuthenticationException CatalogAuthenticationException
221-
* @return A token.
222-
*/
223-
public String createToken(String organizationId, String userId, String secretKey) throws CatalogAuthenticationException {
224-
return createToken(organizationId, userId, Collections.emptyMap(), expiration, secretKey);
193+
return createToken(organizationId, userId, Collections.emptyMap(), expiration);
225194
}
226195

227196
/**
@@ -234,57 +203,7 @@ public String createToken(String organizationId, String userId, String secretKey
234203
* @return A token.
235204
*/
236205
public String createToken(String organizationId, String userId, Map<String, Object> claims) throws CatalogAuthenticationException {
237-
return createToken(organizationId, userId, claims, expiration, (Key) null);
238-
}
239-
240-
/**
241-
* Create a token for the user with default expiration time.
242-
*
243-
* @param organizationId Organization id.
244-
* @param userId user.
245-
* @param claims claims.
246-
* @param secretKey secret key to be used for the token generation.
247-
* @throws CatalogAuthenticationException CatalogAuthenticationException
248-
* @return A token.
249-
*/
250-
public String createToken(String organizationId, String userId, Map<String, Object> claims, String secretKey)
251-
throws CatalogAuthenticationException {
252-
return createToken(organizationId, userId, claims, expiration, secretKey);
253-
}
254-
255-
/**
256-
* Create a token for the user.
257-
*
258-
* @param organizationId Organization id.
259-
* @param userId user.
260-
* @param claims claims.
261-
* @param expiration Expiration time in seconds.
262-
* @throws CatalogAuthenticationException CatalogAuthenticationException
263-
* @return A token.
264-
*/
265-
public String createToken(String organizationId, String userId, Map<String, Object> claims, long expiration)
266-
throws CatalogAuthenticationException {
267-
return createToken(organizationId, userId, claims, expiration, (Key) null);
268-
}
269-
270-
/**
271-
* Create a token for the user.
272-
*
273-
* @param organizationId Organization id.
274-
* @param userId user.
275-
* @param claims claims.
276-
* @param expiration Expiration time in seconds.
277-
* @param secretKey Secret key to be used for the token generation.
278-
* @throws CatalogAuthenticationException CatalogAuthenticationException
279-
* @return A token.
280-
*/
281-
public String createToken(String organizationId, String userId, Map<String, Object> claims, long expiration, String secretKey)
282-
throws CatalogAuthenticationException {
283-
Key privateKey = null;
284-
if (secretKey != null) {
285-
privateKey = converStringToKeyObject(secretKey, jwtManager.getAlgorithm().getJcaName());
286-
}
287-
return createToken(organizationId, userId, claims, expiration, privateKey);
206+
return createToken(organizationId, userId, claims, expiration);
288207
}
289208

290209
/**
@@ -294,11 +213,10 @@ public String createToken(String organizationId, String userId, Map<String, Obje
294213
* @param userId user.
295214
* @param claims claims.
296215
* @param expiration Expiration time in seconds.
297-
* @param secretKey Secret key to be used for the token generation.
298216
* @throws CatalogAuthenticationException CatalogAuthenticationException
299217
* @return A token.
300218
*/
301-
public abstract String createToken(String organizationId, String userId, Map<String, Object> claims, long expiration, Key secretKey)
219+
public abstract String createToken(String organizationId, String userId, Map<String, Object> claims, long expiration)
302220
throws CatalogAuthenticationException;
303221

304222
/**

‎opencga-catalog/src/main/java/org/opencb/opencga/catalog/auth/authentication/AzureADAuthenticationManager.java

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,6 @@
5050
import java.io.IOException;
5151
import java.io.InputStream;
5252
import java.net.URL;
53-
import java.security.Key;
5453
import java.security.PublicKey;
5554
import java.security.cert.CertificateException;
5655
import java.security.cert.CertificateFactory;
@@ -261,12 +260,6 @@ public AuthenticationResponse authenticate(String organizationId, String userId,
261260
}
262261
}
263262

264-
@Override
265-
public AuthenticationResponse authenticate(String organizationId, String userId, String password, String secretKey)
266-
throws CatalogAuthenticationException {
267-
throw new UnsupportedOperationException("AzureAD creates its own tokens. Please, call to the other authenticate method.");
268-
}
269-
270263
@Override
271264
public AuthenticationResponse refreshToken(String refreshToken) throws CatalogAuthenticationException {
272265
AuthenticationContext context;
@@ -429,7 +422,7 @@ public void newPassword(String organizationId, String userId, String newPassword
429422
}
430423

431424
@Override
432-
public String createToken(String organizationId, String userId, Map<String, Object> claims, long expiration, Key secretKey) {
425+
public String createToken(String organizationId, String userId, Map<String, Object> claims, long expiration) {
433426
// Tokens are generated by Azure via authorization code or user-password
434427
throw new UnsupportedOperationException("Tokens are generated by Azure via authorization code or user-password");
435428
}

‎opencga-catalog/src/main/java/org/opencb/opencga/catalog/auth/authentication/CatalogAuthenticationManager.java

Lines changed: 3 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -84,17 +84,6 @@ public AuthenticationResponse authenticate(String organizationId, String userId,
8484
}
8585
}
8686

87-
@Override
88-
public AuthenticationResponse authenticate(String organizationId, String userId, String password, String secretKey)
89-
throws CatalogAuthenticationException {
90-
try {
91-
dbAdaptorFactory.getCatalogUserDBAdaptor(organizationId).authenticate(userId, password);
92-
return new AuthenticationResponse(createToken(organizationId, userId, secretKey));
93-
} catch (CatalogDBException e) {
94-
throw new CatalogAuthenticationException("Could not validate '" + userId + "' password\n" + e.getMessage(), e);
95-
}
96-
}
97-
9887
@Override
9988
public List<User> getUsersFromRemoteGroup(String group) throws CatalogException {
10089
throw new UnsupportedOperationException();
@@ -121,19 +110,18 @@ public void newPassword(String organizationId, String userId, String newPassword
121110
}
122111

123112
@Override
124-
public String createToken(String organizationId, String userId, Map<String, Object> claims, long expiration, Key secretKey)
113+
public String createToken(String organizationId, String userId, Map<String, Object> claims, long expiration)
125114
throws CatalogAuthenticationException {
126115
List<JwtPayload.FederationJwtPayload> federations = getFederations(organizationId, userId);
127116
return jwtManager.createJWTToken(organizationId, AuthenticationOrigin.AuthenticationType.OPENCGA, userId, claims, federations,
128-
secretKey, expiration);
117+
expiration);
129118
}
130119

131120
@Override
132121
public String createNonExpiringToken(String organizationId, String userId, Map<String, Object> claims)
133122
throws CatalogAuthenticationException {
134123
List<JwtPayload.FederationJwtPayload> federations = getFederations(organizationId, userId);
135-
return jwtManager.createJWTToken(organizationId, AuthenticationOrigin.AuthenticationType.OPENCGA, userId, claims, federations,
136-
null, 0L);
124+
return jwtManager.createJWTToken(organizationId, AuthenticationOrigin.AuthenticationType.OPENCGA, userId, claims, federations, 0L);
137125
}
138126

139127
@Override

‎opencga-catalog/src/main/java/org/opencb/opencga/catalog/auth/authentication/JwtManager.java

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -90,8 +90,7 @@ public JwtManager setPublicKey(Key publicKey) {
9090
}
9191

9292
public String createJWTToken(String organizationId, AuthenticationOrigin.AuthenticationType type, String userId,
93-
Map<String, Object> claims, List<JwtPayload.FederationJwtPayload> federations, Key secretKey,
94-
long expiration) {
93+
Map<String, Object> claims, List<JwtPayload.FederationJwtPayload> federations, long expiration) {
9594
long currentTime = System.currentTimeMillis();
9695

9796
JwtBuilder jwtBuilder = Jwts.builder();
@@ -109,7 +108,7 @@ public String createJWTToken(String organizationId, AuthenticationOrigin.Authent
109108
.setAudience(organizationId)
110109
.setIssuer("OpenCGA")
111110
.setIssuedAt(new Date(currentTime))
112-
.signWith(secretKey != null ? secretKey : privateKey, algorithm);
111+
.signWith(privateKey, algorithm);
113112

114113
// Set the expiration in number of seconds only if 'expiration' is greater than 0
115114
if (expiration > 0) {
@@ -123,10 +122,6 @@ public void validateToken(String token) throws CatalogAuthenticationException {
123122
parseClaims(token);
124123
}
125124

126-
public void validateToken(String token, Key publicKey) throws CatalogAuthenticationException {
127-
parseClaims(token, publicKey);
128-
}
129-
130125
public JwtPayload getPayload(String token) throws CatalogAuthenticationException {
131126
Claims body = parseClaims(token).getBody();
132127
return new JwtPayload(body.getSubject(), body.getAudience(), getAuthOrigin(body), body.getIssuer(), body.getIssuedAt(),

‎opencga-catalog/src/main/java/org/opencb/opencga/catalog/auth/authentication/LDAPAuthenticationManager.java

Lines changed: 3 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -171,30 +171,6 @@ public AuthenticationResponse authenticate(String organizationId, String userId,
171171
return new AuthenticationResponse(createToken(organizationId, userId, claims));
172172
}
173173

174-
@Override
175-
public AuthenticationResponse authenticate(String organizationId, String userId, String password, String secretKey)
176-
throws CatalogAuthenticationException {
177-
Map<String, Object> claims = new HashMap<>();
178-
179-
List<Attributes> userInfoFromLDAP = getUserInfoFromLDAP(Arrays.asList(userId), usersSearch);
180-
if (userInfoFromLDAP.isEmpty()) {
181-
throw new CatalogAuthenticationException("LDAP: The user id " + userId + " could not be found.");
182-
}
183-
184-
String rdn = getDN(userInfoFromLDAP.get(0));
185-
claims.put(OPENCGA_DISTINGUISHED_NAME, rdn);
186-
187-
// Attempt to authenticate
188-
Hashtable<String, Object> env = getEnv(rdn, password);
189-
try {
190-
getDirContext(env).close();
191-
} catch (NamingException e) {
192-
throw wrapException(e);
193-
}
194-
195-
return new AuthenticationResponse(createToken(organizationId, userId, claims, secretKey));
196-
}
197-
198174
@Override
199175
public List<User> getUsersFromRemoteGroup(String group) throws CatalogException {
200176
List<String> usersFromLDAP = getUsersFromLDAPGroup(group, groupsSearch);
@@ -261,17 +237,17 @@ public void newPassword(String organizationId, String userId, String newPassword
261237
}
262238

263239
@Override
264-
public String createToken(String organizationId, String userId, Map<String, Object> claims, long expiration, Key secretKey)
240+
public String createToken(String organizationId, String userId, Map<String, Object> claims, long expiration)
265241
throws CatalogAuthenticationException {
266242
List<JwtPayload.FederationJwtPayload> federations = getFederations(organizationId, userId);
267-
return jwtManager.createJWTToken(organizationId, AuthenticationType.LDAP, userId, claims, federations, secretKey, expiration);
243+
return jwtManager.createJWTToken(organizationId, AuthenticationType.LDAP, userId, claims, federations, expiration);
268244
}
269245

270246
@Override
271247
public String createNonExpiringToken(String organizationId, String userId, Map<String, Object> claims)
272248
throws CatalogAuthenticationException {
273249
List<JwtPayload.FederationJwtPayload> federations = getFederations(organizationId, userId);
274-
return jwtManager.createJWTToken(organizationId, AuthenticationType.LDAP, userId, claims, federations, null, 0L);
250+
return jwtManager.createJWTToken(organizationId, AuthenticationType.LDAP, userId, claims, federations, 0L);
275251
}
276252

277253
/* Private methods */

‎opencga-catalog/src/main/java/org/opencb/opencga/catalog/auth/authentication/SSOAuthenticationManager.java

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -42,12 +42,6 @@ public AuthenticationResponse authenticate(String organizationId, String userId,
4242
throw new NotImplementedException("Authentication should be done through SSO");
4343
}
4444

45-
@Override
46-
public AuthenticationResponse authenticate(String organizationId, String userId, String password, String secretKey)
47-
throws CatalogAuthenticationException {
48-
throw new NotImplementedException("Authentication should be done through SSO");
49-
}
50-
5145
@Override
5246
public List<User> getUsersFromRemoteGroup(String group) throws CatalogException {
5347
throw new NotImplementedException("Operation not implemented");
@@ -79,19 +73,18 @@ public void newPassword(String organizationId, String userId, String newPassword
7973
}
8074

8175
@Override
82-
public String createToken(String organizationId, String userId, Map<String, Object> claims, long expiration, Key secretKey)
76+
public String createToken(String organizationId, String userId, Map<String, Object> claims, long expiration)
8377
throws CatalogAuthenticationException {
8478
List<JwtPayload.FederationJwtPayload> federations = getFederations(organizationId, userId);
8579
return jwtManager.createJWTToken(organizationId, AuthenticationOrigin.AuthenticationType.SSO, userId, claims, federations,
86-
secretKey, expiration);
80+
expiration);
8781
}
8882

8983
@Override
9084
public String createNonExpiringToken(String organizationId, String userId, Map<String, Object> claims)
9185
throws CatalogAuthenticationException {
9286
List<JwtPayload.FederationJwtPayload> federations = getFederations(organizationId, userId);
93-
return jwtManager.createJWTToken(organizationId, AuthenticationOrigin.AuthenticationType.SSO, userId, claims, federations, null,
94-
0L);
87+
return jwtManager.createJWTToken(organizationId, AuthenticationOrigin.AuthenticationType.SSO, userId, claims, federations, 0L);
9588
}
9689

9790
@Override

‎opencga-catalog/src/main/java/org/opencb/opencga/catalog/auth/authentication/azure/AuthenticationFactory.java

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -105,28 +105,23 @@ public String createToken(String organizationId, String authOriginId, String use
105105

106106
public void validateToken(String organizationId, Account.AuthenticationOrigin authenticationOrigin, JwtPayload jwtPayload)
107107
throws CatalogException {
108-
String secretKey = null;
108+
String securityKey = null;
109109
if (authenticationOrigin.isFederation()) {
110-
// The user is a federated user, so we need to use a different secret key
111-
secretKey = getFederationSecretKey(organizationId, jwtPayload.getUserId());
110+
// The user is a federated user, so the token should have been encrypted using the security key
111+
securityKey = getFederationSecurityKey(organizationId, jwtPayload.getUserId());
112112
}
113-
getOrganizationAuthenticationManager(organizationId, authenticationOrigin.getId()).validateToken(jwtPayload.getToken(), secretKey);
113+
getOrganizationAuthenticationManager(organizationId, authenticationOrigin.getId()).validateToken(jwtPayload.getToken(),
114+
securityKey);
114115
}
115116

116117
public AuthenticationResponse authenticate(String organizationId, Account.AuthenticationOrigin authenticationOrigin, String userId,
117118
String password) throws CatalogException {
118119
AuthenticationManager organizationAuthenticationManager = getOrganizationAuthenticationManager(organizationId,
119120
authenticationOrigin.getId());
120-
if (authenticationOrigin.isFederation()) {
121-
// The user is a federated user, so we need to use a different secret key
122-
String secretKey = getFederationSecretKey(organizationId, userId);
123-
return organizationAuthenticationManager.authenticate(organizationId, userId, password, secretKey);
124-
} else {
125-
return organizationAuthenticationManager.authenticate(organizationId, userId, password);
126-
}
121+
return organizationAuthenticationManager.authenticate(organizationId, userId, password);
127122
}
128123

129-
private String getFederationSecretKey(String organizationId, String userId) throws CatalogException {
124+
private String getFederationSecurityKey(String organizationId, String userId) throws CatalogException {
130125
QueryOptions options = new QueryOptions(QueryOptions.INCLUDE, OrganizationDBAdaptor.QueryParams.FEDERATION.key());
131126
Organization organization = catalogDBAdaptorFactory.getCatalogOrganizationDBAdaptor(organizationId).get(options).first();
132127
if (organization.getFederation() == null) {

‎opencga-catalog/src/test/java/org/opencb/opencga/catalog/auth/authentication/JwtSessionManagerTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ public void setUp() throws Exception {
5353

5454
@Test
5555
public void testCreateJWTToken() throws Exception {
56-
jwtToken = jwtSessionManager.createJWTToken(organizationId, null, "testUser", Collections.emptyMap(), null, null, 60L);
56+
jwtToken = jwtSessionManager.createJWTToken(organizationId, null, "testUser", Collections.emptyMap(), null, 60L);
5757
}
5858

5959
@Test
@@ -81,7 +81,7 @@ public void testInvalidSecretKey() throws CatalogAuthenticationException {
8181

8282
@Test
8383
public void testNonExpiringToken() throws CatalogException {
84-
String nonExpiringToken = jwtSessionManager.createJWTToken(organizationId, null, "System", null, null, null, -1L);
84+
String nonExpiringToken = jwtSessionManager.createJWTToken(organizationId, null, "System", null, null, -1L);
8585
assertEquals(jwtSessionManager.getUser(nonExpiringToken), "System");
8686
assertNull(jwtSessionManager.getExpiration(nonExpiringToken));
8787
}

0 commit comments

Comments
 (0)