Skip to content

Commit 0b73ab1

Browse files
committed
Fix incomplete API key migration
Fixes #4652 Fixes #4683 Signed-off-by: nscuro <[email protected]>
1 parent 297f3b2 commit 0b73ab1

File tree

12 files changed

+416
-109
lines changed

12 files changed

+416
-109
lines changed

dev/docker-compose.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,9 @@ name: "dependency-track"
1919
services:
2020
apiserver:
2121
image: dependencytrack/apiserver:snapshot
22+
environment:
23+
# Speed up password hashing for faster initial login (default is 14 rounds).
24+
ALPINE_BCRYPT_ROUNDS: "4"
2225
ports:
2326
- "127.0.0.1:8080:8080"
2427
volumes:

src/main/java/org/dependencytrack/resources/v1/TeamResource.java

Lines changed: 26 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@
2323
import alpine.model.ApiKey;
2424
import alpine.model.Team;
2525
import alpine.model.UserPrincipal;
26+
import alpine.security.ApiKeyDecoder;
27+
import alpine.security.InvalidApiKeyFormatException;
2628
import alpine.server.auth.PermissionRequired;
2729
import alpine.server.resources.AlpineResource;
2830
import io.swagger.v3.oas.annotations.Operation;
@@ -36,7 +38,6 @@
3638
import io.swagger.v3.oas.annotations.security.SecurityRequirement;
3739
import io.swagger.v3.oas.annotations.security.SecurityRequirements;
3840
import io.swagger.v3.oas.annotations.tags.Tag;
39-
4041
import org.dependencytrack.auth.Permissions;
4142
import org.dependencytrack.model.validation.ValidUuid;
4243
import org.dependencytrack.persistence.QueryManager;
@@ -59,8 +60,6 @@
5960
import java.util.ArrayList;
6061
import java.util.List;
6162

62-
import static org.datanucleus.PropertyNames.PROPERTY_RETAIN_VALUES;
63-
6463
/**
6564
* JAX-RS resources for processing teams.
6665
*
@@ -308,12 +307,14 @@ public Response regenerateApiKey(
308307
@Parameter(description = "The public ID for the API key or for Legacy the complete Key to regenerate", required = true)
309308
@PathParam("publicIdOrKey") String publicIdOrKey) {
310309
try (QueryManager qm = new QueryManager()) {
311-
boolean isLegacy = publicIdOrKey.length() == ApiKey.LEGACY_FULL_KEY_LENGTH;
312-
ApiKey apiKey;
313-
if (publicIdOrKey.length() == ApiKey.FULL_KEY_LENGTH || isLegacy) {
314-
apiKey = qm.getApiKeyByPublicId(ApiKey.getPublicId(publicIdOrKey, isLegacy));
315-
} else {
316-
apiKey = qm.getApiKeyByPublicId(publicIdOrKey);
310+
ApiKey apiKey = qm.getApiKeyByPublicId(publicIdOrKey);
311+
if (apiKey == null) {
312+
try {
313+
final ApiKey deocdedApiKey = ApiKeyDecoder.decode(publicIdOrKey);
314+
apiKey = qm.getApiKeyByPublicId(deocdedApiKey.getPublicId());
315+
} catch (InvalidApiKeyFormatException e) {
316+
LOGGER.debug("Failed to decode value as API key", e);
317+
}
317318
}
318319
if (apiKey != null) {
319320
apiKey = qm.regenerateApiKey(apiKey);
@@ -349,15 +350,15 @@ public Response updateApiKeyComment(
349350
@PathParam("publicIdOrKey") final String publicIdOrKey,
350351
final String comment) {
351352
try (final var qm = new QueryManager()) {
352-
qm.getPersistenceManager().setProperty(PROPERTY_RETAIN_VALUES, "true");
353-
354353
return qm.callInTransaction(() -> {
355-
boolean isLegacy = publicIdOrKey.length() == ApiKey.LEGACY_FULL_KEY_LENGTH;
356-
ApiKey apiKey;
357-
if (publicIdOrKey.length() == ApiKey.FULL_KEY_LENGTH || isLegacy) {
358-
apiKey = qm.getApiKeyByPublicId(ApiKey.getPublicId(publicIdOrKey, isLegacy));
359-
} else {
360-
apiKey = qm.getApiKeyByPublicId(publicIdOrKey);
354+
ApiKey apiKey = qm.getApiKeyByPublicId(publicIdOrKey);
355+
if (apiKey == null) {
356+
try {
357+
final ApiKey deocdedApiKey = ApiKeyDecoder.decode(publicIdOrKey);
358+
apiKey = qm.getApiKeyByPublicId(deocdedApiKey.getPublicId());
359+
} catch (InvalidApiKeyFormatException e) {
360+
LOGGER.debug("Failed to decode value as API key", e);
361+
}
361362
}
362363
if (apiKey == null) {
363364
return Response
@@ -388,12 +389,14 @@ public Response deleteApiKey(
388389
@Parameter(description = "The public ID for the API key or for Legacy the full Key to delete", required = true)
389390
@PathParam("publicIdOrKey") String publicIdOrKey) {
390391
try (QueryManager qm = new QueryManager()) {
391-
boolean isLegacy = publicIdOrKey.length() == ApiKey.LEGACY_FULL_KEY_LENGTH;
392-
ApiKey apiKey;
393-
if (publicIdOrKey.length() == ApiKey.FULL_KEY_LENGTH || isLegacy) {
394-
apiKey = qm.getApiKeyByPublicId(ApiKey.getPublicId(publicIdOrKey, isLegacy));
395-
} else {
396-
apiKey = qm.getApiKeyByPublicId(publicIdOrKey);
392+
ApiKey apiKey = qm.getApiKeyByPublicId(publicIdOrKey);
393+
if (apiKey == null) {
394+
try {
395+
final ApiKey deocdedApiKey = ApiKeyDecoder.decode(publicIdOrKey);
396+
apiKey = qm.getApiKeyByPublicId(deocdedApiKey.getPublicId());
397+
} catch (InvalidApiKeyFormatException e) {
398+
LOGGER.debug("Failed to decode value as API key", e);
399+
}
397400
}
398401
if (apiKey != null) {
399402
qm.delete(apiKey);
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
/*
2+
* This file is part of Dependency-Track.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*
16+
* SPDX-License-Identifier: Apache-2.0
17+
* Copyright (c) OWASP Foundation. All Rights Reserved.
18+
*/
19+
package org.dependencytrack.upgrade;
20+
21+
import alpine.server.upgrade.UpgradeMetaProcessor;
22+
23+
import java.sql.Connection;
24+
25+
/**
26+
* @since 4.13.0
27+
*/
28+
public interface PreUpgradeHook {
29+
30+
int priority();
31+
32+
boolean shouldExecute(final UpgradeMetaProcessor upgradeProcessor);
33+
34+
void execute(final Connection connection) throws Exception;
35+
36+
}

src/main/java/org/dependencytrack/upgrade/UpgradeInitializer.java

Lines changed: 50 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,15 @@
3838
import jakarta.servlet.ServletContextListener;
3939
import javax.jdo.JDOHelper;
4040
import javax.jdo.PersistenceManager;
41+
import java.sql.Connection;
42+
import java.sql.DriverManager;
43+
import java.sql.SQLException;
44+
import java.util.ArrayList;
45+
import java.util.Comparator;
4146
import java.util.HashSet;
47+
import java.util.List;
4248
import java.util.Properties;
49+
import java.util.ServiceLoader;
4350
import java.util.Set;
4451

4552
public class UpgradeInitializer implements ServletContextListener {
@@ -52,19 +59,32 @@ public class UpgradeInitializer implements ServletContextListener {
5259
@Override
5360
public void contextInitialized(final ServletContextEvent event) {
5461
LOGGER.info("Initializing upgrade framework");
55-
try {
56-
final UpgradeMetaProcessor ump = new UpgradeMetaProcessor();
62+
final var preUpgradeHooks = new ArrayList<PreUpgradeHook>();
63+
try (final UpgradeMetaProcessor ump = new UpgradeMetaProcessor()) {
5764
final VersionComparator currentVersion = ump.getSchemaVersion();
58-
ump.close();
5965
if (currentVersion != null && currentVersion.isOlderThan(new VersionComparator("4.0.0"))) {
6066
LOGGER.error("Unable to upgrade Dependency-Track versions prior to v4.0.0. Please refer to documentation for migration details. Halting.");
67+
ump.close();
6168
Runtime.getRuntime().halt(-1);
6269
}
70+
71+
ServiceLoader.load(PreUpgradeHook.class).stream()
72+
.map(ServiceLoader.Provider::get)
73+
.sorted(Comparator.comparingInt(PreUpgradeHook::priority))
74+
.filter(hook -> hook.shouldExecute(ump))
75+
.forEach(preUpgradeHooks::add);
6376
} catch (UpgradeException e) {
6477
LOGGER.error("An error occurred determining database schema version. Unable to continue.", e);
6578
Runtime.getRuntime().halt(-1);
6679
}
6780

81+
try {
82+
executePreUpgradeHooks(preUpgradeHooks);
83+
} catch (RuntimeException e) {
84+
LOGGER.error("Failed to execute pre-upgrade hooks", e);
85+
Runtime.getRuntime().halt(-1);
86+
}
87+
6888
try (final JDOPersistenceManagerFactory pmf = createPersistenceManagerFactory()) {
6989
// Ensure that the UpgradeMetaProcessor and SchemaVersion tables are created NOW, not dynamically at runtime.
7090
final PersistenceNucleusContext ctx = pmf.getNucleusContext();
@@ -121,4 +141,31 @@ private JDOPersistenceManagerFactory createPersistenceManagerFactory() {
121141
return (JDOPersistenceManagerFactory) JDOHelper.getPersistenceManagerFactory(dnProps, "Alpine");
122142
}
123143

144+
private void executePreUpgradeHooks(final List<PreUpgradeHook> hooks) {
145+
if (hooks.isEmpty()) {
146+
return;
147+
}
148+
149+
final String dbUrl = Config.getInstance().getProperty(Config.AlpineKey.DATABASE_URL);
150+
final String dbUser = Config.getInstance().getProperty(Config.AlpineKey.DATABASE_USERNAME);
151+
final String dbPassword = Config.getInstance().getPropertyOrFile(Config.AlpineKey.DATABASE_PASSWORD);
152+
try (final Connection connection = DriverManager.getConnection(dbUrl, dbUser, dbPassword)) {
153+
connection.setAutoCommit(false);
154+
155+
for (final PreUpgradeHook hook : hooks) {
156+
LOGGER.info("Executing pre-upgrade hook: " + hook.getClass().getName());
157+
try {
158+
hook.execute(connection);
159+
connection.commit();
160+
} catch (Exception e) {
161+
throw new IllegalStateException("Failed to execute pre-upgrade hook: " + hook.getClass().getName(), e);
162+
} finally {
163+
connection.rollback();
164+
}
165+
}
166+
} catch (SQLException e) {
167+
throw new IllegalStateException("Failed to create database connection", e);
168+
}
169+
}
170+
124171
}

src/main/java/org/dependencytrack/upgrade/UpgradeItems.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ class UpgradeItems {
4343
UPGRADE_ITEMS.add(org.dependencytrack.upgrade.v4122.v4122Updater.class);
4444
UPGRADE_ITEMS.add(org.dependencytrack.upgrade.v4123.v4123Updater.class);
4545
UPGRADE_ITEMS.add(org.dependencytrack.upgrade.v4130.v4130Updater.class);
46+
UPGRADE_ITEMS.add(org.dependencytrack.upgrade.v4130.v4130_1Updater.class);
4647
}
4748

4849
static List<Class<? extends UpgradeItem>> getUpgradeItems() {
Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
/*
2+
* This file is part of Dependency-Track.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*
16+
* SPDX-License-Identifier: Apache-2.0
17+
* Copyright (c) OWASP Foundation. All Rights Reserved.
18+
*/
19+
package org.dependencytrack.upgrade.v4130;
20+
21+
import alpine.common.logging.Logger;
22+
import alpine.common.util.VersionComparator;
23+
import alpine.server.upgrade.UpgradeMetaProcessor;
24+
import org.dependencytrack.upgrade.PreUpgradeHook;
25+
26+
import java.sql.Connection;
27+
import java.sql.PreparedStatement;
28+
import java.sql.Statement;
29+
30+
/**
31+
* Hook to create the {@code APIKEY.PUBLIC_ID} column and accompanying {@code UNIQUE} index
32+
* for MSSQL <em>before</em> the main schema migration is executed. The index is partial
33+
* to allow for multiple {@code NULL} values, which is necessary for the duration of the
34+
* API key migration performed in {@link v4130_1Updater}.
35+
*
36+
* @see <a href="https://github.com/DependencyTrack/dependency-track/issues/4683">GitHub Issue</a>
37+
* @since 4.13.0
38+
*/
39+
public class v4130PreUpgradeHook implements PreUpgradeHook {
40+
41+
private static final Logger LOGGER = Logger.getLogger(v4130PreUpgradeHook.class);
42+
43+
@Override
44+
public int priority() {
45+
return 1;
46+
}
47+
48+
@Override
49+
public boolean shouldExecute(final UpgradeMetaProcessor upgradeProcessor) {
50+
final VersionComparator currentSchemaVersion = upgradeProcessor.getSchemaVersion();
51+
return currentSchemaVersion != null && currentSchemaVersion.isOlderThan(new VersionComparator("4.13.0"));
52+
}
53+
54+
@Override
55+
public void execute(final Connection connection) throws Exception {
56+
if (!connection.getMetaData().getDatabaseProductName().equals("Microsoft SQL Server")) {
57+
LOGGER.info("Database is not MSSQL; Nothing to do");
58+
return;
59+
}
60+
61+
try (final PreparedStatement ps = connection.prepareStatement("""
62+
SELECT 1
63+
FROM INFORMATION_SCHEMA.COLUMNS
64+
WHERE UPPER(TABLE_NAME) = 'APIKEY'
65+
AND UPPER(COLUMN_NAME) = 'PUBLIC_ID'
66+
""")) {
67+
if (ps.executeQuery().next()) {
68+
LOGGER.info("PUBLIC_ID column already exists in APIKEY table; Nothing to do");
69+
return;
70+
}
71+
}
72+
73+
try (final Statement stmt = connection.createStatement()) {
74+
stmt.execute("ALTER TABLE \"APIKEY\" ADD \"PUBLIC_ID\" VARCHAR(8) NULL");
75+
stmt.execute("CREATE UNIQUE INDEX \"APIKEY_PUBLIC_IDX\" ON \"APIKEY\"(\"PUBLIC_ID\") WHERE \"PUBLIC_ID\" IS NOT NULL");
76+
}
77+
}
78+
79+
}

src/main/java/org/dependencytrack/upgrade/v4130/v4130Updater.java

Lines changed: 8 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -18,76 +18,26 @@
1818
*/
1919
package org.dependencytrack.upgrade.v4130;
2020

21-
import java.security.MessageDigest;
22-
import java.sql.Connection;
23-
import java.sql.PreparedStatement;
24-
import java.sql.ResultSet;
25-
import java.sql.Statement;
26-
import java.util.HexFormat;
27-
28-
import alpine.common.logging.Logger;
29-
import alpine.model.ApiKey;
3021
import alpine.persistence.AlpineQueryManager;
3122
import alpine.server.upgrade.AbstractUpgradeItem;
32-
import alpine.server.util.DbUtil;
3323

34-
public class v4130Updater extends AbstractUpgradeItem {
24+
import java.sql.Connection;
3525

36-
private static final Logger LOGGER = Logger.getLogger(v4130Updater.class);
26+
public class v4130Updater extends AbstractUpgradeItem {
3727

3828
@Override
3929
public String getSchemaVersion() {
4030
return "4.13.0";
4131
}
4232

4333
@Override
44-
public void executeUpgrade(final AlpineQueryManager qm, final Connection connection) throws Exception {
45-
migrateToHashedApiKey(connection);
34+
public boolean shouldUpgrade(final AlpineQueryManager qm, final Connection connection) {
35+
return false; // Revised upgrade logic moved to v4130_1Updater.
4636
}
4737

48-
private void migrateToHashedApiKey(final Connection connection) throws Exception {
49-
LOGGER.info("Store API keys in hashed format!");
50-
51-
try (final PreparedStatement ps = connection.prepareStatement("""
52-
UPDATE "APIKEY"
53-
SET "APIKEY" = ?, "PUBLIC_ID" = ?, "IS_LEGACY" = ?
54-
WHERE "ID" = ?
55-
""")) {
56-
57-
if (DbUtil.isMysql() || DbUtil.isMssql()) {
58-
ps.setInt(3, 1);
59-
} else {
60-
ps.setBoolean(3, true);
61-
}
62-
63-
try (final Statement statement = connection.createStatement()) {
64-
statement.execute("""
65-
SELECT "ID", "APIKEY"
66-
FROM "APIKEY"
67-
""");
68-
try (final ResultSet rs = statement.getResultSet()) {
69-
String clearKey;
70-
int id;
71-
String hashedKey;
72-
String publicId;
73-
while (rs.next()) {
74-
clearKey = rs.getString("apikey");
75-
if (clearKey.length() != ApiKey.LEGACY_FULL_KEY_LENGTH) {
76-
continue;
77-
}
78-
final MessageDigest digest = MessageDigest.getInstance("SHA3-256");
79-
id = rs.getInt("id");
80-
hashedKey = HexFormat.of().formatHex(digest.digest(ApiKey.getOnlyKeyAsBytes(clearKey, true)));
81-
publicId = ApiKey.getPublicId(clearKey, true);
82-
83-
ps.setString(1, hashedKey);
84-
ps.setString(2, publicId);
85-
ps.setInt(4, id);
86-
87-
ps.executeUpdate();
88-
}
89-
}
90-
}
91-
}
38+
@Override
39+
public void executeUpgrade(final AlpineQueryManager qm, final Connection connection) throws Exception {
40+
// Revised upgrade logic moved to v4130_1Updater.
9241
}
42+
9343
}

0 commit comments

Comments
 (0)