Skip to content

Commit 611949d

Browse files
authored
Merge commit from fork
* fix regex checking to use find instead of matches * cleanup regex handling * add argon2 hashing for api keys * add api key version prefix in order to establish transition strategy fix update to do a delete+insert to match schema * update to not hash the first 12 characters of the apikey this allows for quicker filtering on read and only needing to check hashes that match * split prefix into separately randomly generated string * fix keyId flipflop * fix off-by-one error * add backwards compatibility support enabled by feature flag
1 parent 7da86e2 commit 611949d

9 files changed

Lines changed: 140 additions & 58 deletions

File tree

compose_files/sql/users.sql

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
set define on
22
define OFFICE_EROC=&1
3-
defin API_KEY=&2
43
begin
54
cwms_sec.add_user_to_group('&&OFFICE_EROC.webtest', 'All Users', 'HQ');
65
cwms_sec.add_user_to_group('&&OFFICE_EROC.webtest', 'All Users', 'SPK');
@@ -36,17 +35,6 @@ begin
3635
cwms_sec.add_user_to_group('q0hectest', 'CWMS PD Users', 'LRL');
3736
cwms_sec.add_user_to_group('q0hectest', 'TS ID Creator', 'LRL');
3837
execute immediate 'grant execute on cwms_20.cwms_upass to web_user';
39-
insert into "CWMS_20"."AT_API_KEYS" (
40-
userid,
41-
key_name,
42-
apikey,
43-
created,
44-
expires
45-
) values ( 'Q0HECTEST',
46-
'test',
47-
'&&API_KEY',
48-
to_date('2025-06-10 16:10:42','YYYY-MM-DD HH24:MI:SS'),
49-
to_date('2029-06-16 16:10:46','YYYY-MM-DD HH24:MI:SS') );
5038

5139
cwms_sec.add_cwms_user('m5hectest',NULL,'SWT');
5240
cwms_sec.add_user_to_group('m5hectest','All Users', 'SWT');

cwms-data-api/build.gradle

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ dependencies {
129129
implementation(libs.swagger.core) {
130130
exclude group: "jakarta.xml.bind", module: "*"
131131
}
132-
132+
implementation(libs.password4j)
133133
implementation 'cz.jirutka.rsql:rsql-parser:2.1.0'
134134

135135
compileOnly(libs.javaee.web.api)
@@ -272,6 +272,7 @@ task run(type: JavaExec) {
272272
jvmArgs += "-Dorg.apache.tomcat.util.digester.PROPERTY_SOURCE=org.apache.tomcat.util.digester.EnvironmentPropertySource"
273273
jvmArgs += "-Dcatalina.base=$buildDir/tomcat"
274274
jvmArgs += "-DwarContext=/" + context
275+
jvmArgs += "-DAUTH_RE_ENABLE_NON_HASH_KEY_SUPPORT=true"
275276
// If you have the docker-compose environment up and are trying to run
276277
// CDA from run to debug uncomment the following lines.
277278
//jvmArgs += "-Dcwms.dataapi.access.providers=KeyAccessManager,CwmsAccessManager,OpenID"

cwms-data-api/src/main/java/cwms/cda/data/dao/AuthDao.java

Lines changed: 98 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package cwms.cda.data.dao;
22

33
import com.google.common.flogger.FluentLogger;
4+
import com.password4j.Hash;
5+
import com.password4j.HashUpdate;
46
import cwms.cda.ApiServlet;
57
import cwms.cda.data.dto.auth.ApiKey;
68
import cwms.cda.datasource.ConnectionPreparer;
@@ -10,14 +12,17 @@
1012
import cwms.cda.datasource.SessionOfficePreparer;
1113
import cwms.cda.datasource.SessionTimeZonePreparer;
1214
import cwms.cda.helpers.ResourceHelper;
15+
import cwms.cda.features.CdaFeatures;
1316
import cwms.cda.security.CwmsAuthException;
1417
import cwms.cda.security.DataApiPrincipal;
1518
import cwms.cda.security.MissingRolesException;
1619
import cwms.cda.security.Role;
1720
import io.javalin.core.security.RouteRole;
1821
import io.javalin.http.Context;
1922
import io.javalin.http.HttpCode;
23+
import org.togglz.core.context.FeatureContext;
2024

25+
import com.password4j.Password;
2126
import java.security.NoSuchAlgorithmException;
2227
import java.security.SecureRandom;
2328
import java.sql.Connection;
@@ -56,6 +61,8 @@ public class AuthDao extends Dao<DataApiPrincipal> {
5661
+ "23.03.16 or later to handle authorization operations.";
5762
public static final String DATA_API_PRINCIPAL = "DataApiPrincipal";
5863
public static final String AUTH_ERROR_MSG = "Authentication failed. The API Key may be invalid or no longer active.";
64+
private static final String API_KEY_V1_PREFIX = "ak1_";
65+
private static final int API_KEY_ID_LENGTH = 12;
5966
// At this level we just care that the user has permissions in *any* office
6067
private static final String RETRIEVE_GROUPS_OF_USER =
6168
ResourceHelper.getResourceAsString("/cwms/data/sql/user_groups.sql", AuthDao.class);
@@ -68,7 +75,8 @@ public class AuthDao extends Dao<DataApiPrincipal> {
6875
+ "cwms_env.set_session_user_direct(upper(?),upper(?)); end;";
6976

7077
private static final String CHECK_API_KEY =
71-
"select userid from cwms_20.at_api_keys where apikey = ? and (expires is null or expires >= systimestamp)";
78+
"select userid, apikey, key_name, expires from cwms_20.at_api_keys where (expires is null or expires >= systimestamp) " +
79+
"and apikey like ?";
7280

7381
private static final String USER_FOR_EDIPI =
7482
"select userid from cwms_20.at_sec_cwms_users where edipi = ?";
@@ -194,30 +202,82 @@ static void setSessionForAuthCheck(Connection conn) throws SQLException {
194202
}
195203
}
196204

205+
private static String hashApiKey(String apiKey) {
206+
return Password.hash(apiKey)
207+
.withArgon2()
208+
.getResult();
209+
}
210+
197211
private String checkKey(String key) throws CwmsAuthException {
198212
try {
199213
return dsl.connectionResult(c -> {
200214
setSessionForAuthCheck(c);
201-
try (PreparedStatement checkForKey = c.prepareStatement(CHECK_API_KEY)) {
202-
checkForKey.setString(1,key);
203-
try (ResultSet rs = checkForKey.executeQuery()) {
204-
if (rs.next()) {
205-
return rs.getString(1);
206-
} else {
207-
throw new CwmsAuthException(AUTH_ERROR_MSG);
215+
216+
boolean legacySupport = FeatureContext.getFeatureManager()
217+
.isActive(CdaFeatures.AUTH_RE_ENABLE_NON_HASH_KEY_SUPPORT);
218+
219+
if (key.startsWith(API_KEY_V1_PREFIX) && key.length() > API_KEY_ID_LENGTH) {
220+
try (PreparedStatement checkForKey = c.prepareStatement(CHECK_API_KEY)) {
221+
String keyId = key.substring(0, API_KEY_ID_LENGTH);
222+
checkForKey.setString(1, keyId + "%");
223+
try (ResultSet rs = checkForKey.executeQuery()) {
224+
if (rs.next()) {
225+
String secretKey = key.substring(API_KEY_ID_LENGTH);
226+
String persistentHash = rs.getString(2).substring(API_KEY_ID_LENGTH);
227+
HashUpdate hashUpdate = checkKey(secretKey, persistentHash);
228+
if (hashUpdate.isVerified()) {
229+
String userId = rs.getString(1);
230+
if (hashUpdate.isUpdated()) {
231+
Hash newHash = hashUpdate.getHash();
232+
String newHashedApiKey = keyId + newHash.getResult();
233+
String keyName = rs.getString(3);
234+
Date expires = rs.getDate(4);
235+
updateHash(c, userId, keyName, expires, newHashedApiKey);
236+
}
237+
return userId;
238+
}
239+
}
240+
}
241+
}
242+
} else if (legacySupport) {
243+
try (PreparedStatement checkForKey = c.prepareStatement(CHECK_API_KEY)) {
244+
checkForKey.setString(1, key);
245+
try (ResultSet rs = checkForKey.executeQuery()) {
246+
if (rs.next()) {
247+
return rs.getString(1);
248+
}
208249
}
209250
}
210-
} catch (SQLException ex) {
211-
throw new CwmsAuthException("Failed API key check",ex);
212251
}
252+
throw new CwmsAuthException(AUTH_ERROR_MSG);
213253
});
214-
} catch (DataAccessException ex) {
215-
Throwable t = ex.getCause();
216-
if (t instanceof CwmsAuthException) {
217-
throw (CwmsAuthException)t;
218-
} else {
219-
throw ex;
220-
}
254+
} catch (RuntimeException ex) {
255+
// Don't expose internal database errors
256+
logger.atWarning().withCause(ex).log("Error verifying API key.");
257+
throw new CwmsAuthException(AUTH_ERROR_MSG);
258+
}
259+
}
260+
261+
private static HashUpdate checkKey(String keyFromClient, String persistentHash) {
262+
return Password.check(keyFromClient, persistentHash)
263+
.andUpdate()
264+
.withArgon2();
265+
}
266+
267+
private void updateHash(Connection c, String userId, String keyName, Date expires, String apiKey)
268+
throws SQLException {
269+
//Schema does not allow for row updates. Delete + recreate
270+
try (PreparedStatement deleteKey = c.prepareStatement(REMOVE_API_KEY)) {
271+
deleteKey.setString(1, userId);
272+
deleteKey.setString(2, keyName);
273+
deleteKey.execute();
274+
}
275+
try (PreparedStatement createKey = c.prepareStatement(CREATE_API_KEY)) {
276+
createKey.setString(1, userId);
277+
createKey.setString(2, keyName);
278+
createKey.setString(3, apiKey);
279+
createKey.setDate(4, expires);
280+
createKey.execute();
221281
}
222282
}
223283

@@ -412,15 +472,13 @@ public ApiKey createApiKey(DataApiPrincipal p, ApiKey sourceData) throws CwmsAut
412472
throw new CwmsAuthException(ONLY_OWN_KEY_MESSAGE, HttpCode.UNAUTHORIZED.getStatus());
413473
}
414474
SecureRandom randomSource = SecureRandom.getInstanceStrong();
415-
String key = randomSource.ints((char)'0',(char)'z') // allow a-zA-Z0-9
416-
.filter(i -> (i <= 57 || i >= 65) && (i <= 90 || i >= 97)) // actually filter to above
417-
.limit(256)
418-
.collect(StringBuilder::new,StringBuilder::appendCodePoint, StringBuilder::append)
419-
.toString();
475+
String secretKey = generateSecretKey(randomSource);
476+
String keyId = generateKeyId(randomSource);
477+
String fullApiKey = keyId + secretKey;
420478
final ApiKey newKey = new ApiKey(
421479
sourceData.getUserId().toUpperCase(),
422480
sourceData.getKeyName(),
423-
key,
481+
fullApiKey,
424482
ZonedDateTime.now(ZoneId.of("UTC")),
425483
sourceData.getExpires()
426484
);
@@ -429,7 +487,7 @@ public ApiKey createApiKey(DataApiPrincipal p, ApiKey sourceData) throws CwmsAut
429487
try (PreparedStatement createKey = c.prepareStatement(CREATE_API_KEY)) {
430488
createKey.setString(1, newKey.getUserId());
431489
createKey.setString(2, newKey.getKeyName());
432-
createKey.setString(3, newKey.getApiKey());
490+
createKey.setString(3, keyId + hashApiKey(secretKey));
433491
createKey.setDate(4, new Date(newKey.getCreated().toInstant().toEpochMilli()),
434492
Calendar.getInstance(TimeZone.getTimeZone("UTC")));
435493
if (newKey.getExpires() != null) {
@@ -453,6 +511,22 @@ public ApiKey createApiKey(DataApiPrincipal p, ApiKey sourceData) throws CwmsAut
453511

454512
}
455513

514+
private static String generateSecretKey(SecureRandom randomSource) {
515+
return randomSource.ints('0', 'z') // allow a-zA-Z0-9
516+
.filter(i -> (i <= 57 || i >= 65) && (i <= 90 || i >= 97)) // actually filter to above
517+
.limit(256)
518+
.collect(StringBuilder::new, StringBuilder::appendCodePoint, StringBuilder::append)
519+
.toString();
520+
}
521+
522+
private static String generateKeyId(SecureRandom randomSource) {
523+
return API_KEY_V1_PREFIX + randomSource.ints('0', 'z') // allow a-zA-Z0-9
524+
.filter(i -> (i <= 57 || i >= 65) && (i <= 90 || i >= 97)) // actually filter to above
525+
.limit(API_KEY_ID_LENGTH - API_KEY_V1_PREFIX.length())
526+
.collect(StringBuilder::new, StringBuilder::appendCodePoint, StringBuilder::append)
527+
.toString();
528+
}
529+
456530
/**
457531
* Return all API Keys for a given user.
458532
* @param p User for which we want the keys

cwms-data-api/src/main/java/cwms/cda/features/CdaFeatures.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,5 +5,7 @@
55

66
public enum CdaFeatures implements Feature {
77
@Label("Use object-storage backed Blob DAO in BlobController")
8-
USE_OBJECT_STORAGE_BLOBS
8+
USE_OBJECT_STORAGE_BLOBS,
9+
@Label("Re-enable non-hash key support")
10+
AUTH_RE_ENABLE_NON_HASH_KEY_SUPPORT
911
}
Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1,2 @@
1-
USE_OBJECT_STORAGE_BLOBS=false
1+
USE_OBJECT_STORAGE_BLOBS=false
2+
AUTH_RE_ENABLE_NON_HASH_KEY_SUPPORT=false

cwms-data-api/src/test/java/cwms/cda/api/DataApiTestIT.java

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -29,16 +29,19 @@
2929

3030
import com.atlassian.oai.validator.restassured.OpenApiValidationFilter;
3131
import com.google.common.flogger.FluentLogger;
32+
import cwms.cda.data.dao.AuthDao;
3233
import cwms.cda.data.dao.DeleteRule;
3334
import cwms.cda.data.dao.StreamDao;
3435
import cwms.cda.data.dao.VerticalDatum;
3536
import cwms.cda.data.dao.basin.BasinDao;
3637
import cwms.cda.data.dto.Location;
3738
import cwms.cda.data.dto.LocationCategory;
3839
import cwms.cda.data.dto.LocationGroup;
40+
import cwms.cda.data.dto.auth.ApiKey;
3941
import cwms.cda.data.dto.basin.Basin;
4042
import cwms.cda.data.dto.stream.Stream;
4143
import cwms.cda.helpers.ZoneIdHelper;
44+
import cwms.cda.security.DataApiPrincipal;
4245
import fixtures.CwmsDataApiSetupCallback;
4346
import fixtures.IntegrationTestNameGenerator;
4447
import fixtures.KeyCloakExtension;
@@ -59,11 +62,13 @@
5962
import java.sql.PreparedStatement;
6063
import java.sql.ResultSet;
6164
import java.sql.SQLException;
65+
import java.time.ZonedDateTime;
6266
import java.util.ArrayList;
6367
import java.util.Collections;
6468
import java.util.HashMap;
6569
import java.util.List;
6670
import java.util.Map;
71+
import java.util.Set;
6772
import java.util.function.Consumer;
6873
import mil.army.usace.hec.test.database.CwmsDatabaseContainer;
6974
import org.apache.catalina.Manager;
@@ -99,8 +104,7 @@ public class DataApiTestIT {
99104
protected static String createLocationQuery = null;
100105
protected static String createTimeseriesQuery = null;
101106
protected static String createTimeseriesOffsetQuery = null;
102-
protected static final String registerApiKey = "insert into at_api_keys(userid,key_name,apikey,expires) values(UPPER(?),?,?, null)";
103-
protected static final String removeApiKeys = "delete from at_api_keys where UPPER(userid) = UPPER(?) and apikey = ?";
107+
protected static final String removeApiKeys = "delete from at_api_keys where UPPER(userid) = UPPER(?) and key_name = ?";
104108

105109
protected static final Configuration freemarkerConfig = new Configuration(Configuration.VERSION_2_3_32);
106110

@@ -182,7 +186,7 @@ public static void register_users() throws Exception {
182186
final Manager tsm = CwmsDataApiSetupCallback.getTestSessionManager();
183187
CwmsDatabaseContainer<?> db = CwmsDataApiSetupCallback.getDatabaseLink();
184188
for (TestAccounts.KeyUser user : TestAccounts.KeyUser.values()) {
185-
if (user.getApikey() == null) {
189+
if (user.getKeyName() == null) {
186190
continue;
187191
}
188192
if (user == TestAccounts.KeyUser.SPK_OTHER_NORMAL_SAME_ROLES) {
@@ -207,14 +211,11 @@ public static void register_users() throws Exception {
207211
}
208212

209213
db.connection((c) -> {
210-
try (PreparedStatement stmt = c.prepareStatement(registerApiKey)) {
211-
stmt.setString(1, user.getName());
212-
stmt.setString(2, user.getName() + "TestKey");
213-
stmt.setString(3, user.getApikey());
214-
stmt.execute();
215-
} catch (SQLException ex) {
216-
throw new RuntimeException("Unable to register user:" + user.getName(), ex);
217-
}
214+
String key = AuthDao.getInstance(DSL.using(c), null)
215+
.createApiKey(new DataApiPrincipal(user.getName(), Set.of()),
216+
new ApiKey(user.getName(), user.getKeyName(), null,
217+
ZonedDateTime.now(), null)).getApiKey();
218+
user.setApiKey(key);
218219
}, "cwms_20");
219220

220221
StandardSession session = (StandardSession) tsm.createSession(user.getJSessionId());
@@ -258,7 +259,7 @@ public static void deregister_users() throws Exception {
258259
db.connection((c) -> {
259260
try (PreparedStatement stmt = c.prepareStatement(removeApiKeys)) {
260261
stmt.setString(1, user.getName());
261-
stmt.setString(2, user.getApikey());
262+
stmt.setString(2, user.getKeyName());
262263
stmt.execute();
263264
} catch (SQLException ex) {
264265
throw new RuntimeException("Unable to delete api key", ex);

cwms-data-api/src/test/java/cwms/cda/features/CdaFeatureManagerProviderTest.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,8 @@ void testUseObjectStorageBlobsFeature() throws IOException {
5050
tempFile.deleteOnExit();
5151

5252
try (FileWriter writer = new FileWriter(tempFile)) {
53-
writer.write(CdaFeatures.USE_OBJECT_STORAGE_BLOBS.name() + " = true");
53+
writer.write(CdaFeatures.USE_OBJECT_STORAGE_BLOBS.name() + " = true" + System.lineSeparator());
54+
writer.write(CdaFeatures.AUTH_RE_ENABLE_NON_HASH_KEY_SUPPORT.name() + " = true");
5455
}
5556

5657
System.setProperty(CdaFeatureManagerProvider.PROPERTIES_FILE, tempFile.getAbsolutePath());
@@ -59,6 +60,7 @@ void testUseObjectStorageBlobsFeature() throws IOException {
5960
FeatureManager manager = provider.getFeatureManager();
6061

6162
assertTrue(manager.isActive(CdaFeatures.USE_OBJECT_STORAGE_BLOBS));
63+
assertTrue(manager.isActive(CdaFeatures.AUTH_RE_ENABLE_NON_HASH_KEY_SUPPORT));
6264
}
6365

6466
@Test
@@ -73,5 +75,6 @@ void testFeatureDisabledByDefault() throws IOException {
7375
FeatureManager manager = provider.getFeatureManager();
7476

7577
assertFalse(manager.isActive(CdaFeatures.USE_OBJECT_STORAGE_BLOBS));
78+
assertFalse(manager.isActive(CdaFeatures.AUTH_RE_ENABLE_NON_HASH_KEY_SUPPORT));
7679
}
7780
}

0 commit comments

Comments
 (0)