diff --git a/git-proxy-java-core/src/main/java/org/finos/gitproxy/user/CompositeUserStore.java b/git-proxy-java-core/src/main/java/org/finos/gitproxy/user/CompositeUserStore.java index 469755f5..a3cdbdfd 100644 --- a/git-proxy-java-core/src/main/java/org/finos/gitproxy/user/CompositeUserStore.java +++ b/git-proxy-java-core/src/main/java/org/finos/gitproxy/user/CompositeUserStore.java @@ -184,6 +184,11 @@ public void upsertUser(String username) { mutableStore.upsertUser(username); } + @Override + public void upsertUser(String username, List roles) { + mutableStore.upsertUser(username, roles); + } + @Override public void upsertLockedEmail(String username, String email, String authSource) { mutableStore.upsertLockedEmail(username, email, authSource); diff --git a/git-proxy-java-core/src/main/java/org/finos/gitproxy/user/JdbcUserStore.java b/git-proxy-java-core/src/main/java/org/finos/gitproxy/user/JdbcUserStore.java index 0858ec9b..d4176f77 100644 --- a/git-proxy-java-core/src/main/java/org/finos/gitproxy/user/JdbcUserStore.java +++ b/git-proxy-java-core/src/main/java/org/finos/gitproxy/user/JdbcUserStore.java @@ -208,14 +208,25 @@ public void removeScmIdentity(String username, String provider, String scmUserna * The password is left NULL so the account cannot be used for form login. */ public void upsertUser(String username) { + upsertUser(username, List.of("USER")); + } + + @Override + public void upsertUser(String username, List roles) { + String rolesStr = roles.isEmpty() ? "USER" : String.join(",", roles); boolean exists = !jdbc.queryForList( "SELECT username FROM proxy_users WHERE username = :u", Map.of("u", username), String.class) .isEmpty(); if (!exists) { jdbc.update( - "INSERT INTO proxy_users (username, password_hash, roles) VALUES (:u, NULL, 'USER')", - Map.of("u", username)); - log.debug("Auto-provisioned IdP user '{}'", username); + "INSERT INTO proxy_users (username, password_hash, roles) VALUES (:u, NULL, :roles)", + Map.of("u", username, "roles", rolesStr)); + log.debug("Auto-provisioned IdP user '{}' with roles {}", username, rolesStr); + } else { + jdbc.update( + "UPDATE proxy_users SET roles = :roles WHERE username = :u", + Map.of("u", username, "roles", rolesStr)); + log.debug("Synced IdP roles for user '{}': {}", username, rolesStr); } } diff --git a/git-proxy-java-core/src/main/java/org/finos/gitproxy/user/MongoUserStore.java b/git-proxy-java-core/src/main/java/org/finos/gitproxy/user/MongoUserStore.java index c275e8ea..6b379a80 100644 --- a/git-proxy-java-core/src/main/java/org/finos/gitproxy/user/MongoUserStore.java +++ b/git-proxy-java-core/src/main/java/org/finos/gitproxy/user/MongoUserStore.java @@ -157,14 +157,23 @@ public void setPassword(String username, String passwordHash) { @Override public void upsertUser(String username) { + upsertUser(username, List.of("USER")); + } + + @Override + public void upsertUser(String username, List roles) { + String rolesStr = roles.isEmpty() ? "USER" : String.join(",", roles); if (getCollection().find(Filters.eq("_id", username)).first() == null) { getCollection() .insertOne(new Document("_id", username) .append("passwordHash", null) - .append("roles", "USER") + .append("roles", rolesStr) .append("emails", List.of()) .append("scmIdentities", List.of())); - log.debug("Auto-provisioned IdP user '{}'", username); + log.debug("Auto-provisioned IdP user '{}' with roles {}", username, rolesStr); + } else { + getCollection().updateOne(Filters.eq("_id", username), Updates.set("roles", rolesStr)); + log.debug("Synced IdP roles for user '{}': {}", username, rolesStr); } } diff --git a/git-proxy-java-core/src/main/java/org/finos/gitproxy/user/UserStore.java b/git-proxy-java-core/src/main/java/org/finos/gitproxy/user/UserStore.java index 6ea10946..abee8b51 100644 --- a/git-proxy-java-core/src/main/java/org/finos/gitproxy/user/UserStore.java +++ b/git-proxy-java-core/src/main/java/org/finos/gitproxy/user/UserStore.java @@ -62,6 +62,14 @@ public interface UserStore extends ReadOnlyUserStore { */ void upsertUser(String username); + /** + * Ensures a user row exists and syncs the given roles on every IdP login. Roles are authoritative from the IdP — + * any existing roles are overwritten so that IdP group changes take effect on next sign-in. + */ + default void upsertUser(String username, List roles) { + upsertUser(username); + } + /** Inserts or updates an email for a user as locked (owned by the identity provider). */ void upsertLockedEmail(String username, String email, String authSource); diff --git a/git-proxy-java-core/src/test/java/org/finos/gitproxy/user/JdbcUserStoreIntegrationTest.java b/git-proxy-java-core/src/test/java/org/finos/gitproxy/user/JdbcUserStoreIntegrationTest.java index ecaea950..18241ef0 100644 --- a/git-proxy-java-core/src/test/java/org/finos/gitproxy/user/JdbcUserStoreIntegrationTest.java +++ b/git-proxy-java-core/src/test/java/org/finos/gitproxy/user/JdbcUserStoreIntegrationTest.java @@ -247,6 +247,38 @@ void upsertUser_preservesExistingUser() { assertEquals("{noop}pw", result.get().getPasswordHash()); } + @Test + void upsertUser_withRoles_setsRolesOnNewUser() { + store.upsertUser("oidcuser", List.of("USER", "ADMIN")); + + var result = store.findByUsername("oidcuser"); + assertTrue(result.isPresent()); + assertNull(result.get().getPasswordHash()); + assertTrue(result.get().getRoles().contains("ADMIN")); + assertTrue(result.get().getRoles().contains("USER")); + } + + @Test + void upsertUser_withRoles_syncsRolesOnSubsequentLogin() { + store.upsertUser("oidcuser", List.of("USER")); + store.upsertUser("oidcuser", List.of("USER", "SELF_CERTIFY")); + + var result = store.findByUsername("oidcuser"); + assertTrue(result.isPresent()); + assertTrue(result.get().getRoles().contains("SELF_CERTIFY")); + } + + @Test + void upsertUser_withRoles_syncsRolesForYamlUser() { + store.upsertAll(List.of(user("alice", List.of(), List.of()))); + store.upsertUser("alice", List.of("USER", "ADMIN")); + + var result = store.findByUsername("alice"); + assertTrue(result.isPresent()); + assertEquals("{noop}pw", result.get().getPasswordHash()); + assertTrue(result.get().getRoles().contains("ADMIN")); + } + // ---- upsertLockedEmail ---- @Test diff --git a/git-proxy-java-dashboard/frontend/src/pages/PushDetail.tsx b/git-proxy-java-dashboard/frontend/src/pages/PushDetail.tsx index 294323eb..f17822f2 100644 --- a/git-proxy-java-dashboard/frontend/src/pages/PushDetail.tsx +++ b/git-proxy-java-dashboard/frontend/src/pages/PushDetail.tsx @@ -100,7 +100,6 @@ function formatTime(ts: string | number | undefined) { } } - /** * Build a direct link to the commit on the upstream SCM. * GitHub / Gitea / Codeberg: {repo}/commit/{sha} diff --git a/git-proxy-java-dashboard/src/main/java/org/finos/gitproxy/dashboard/SecurityConfig.java b/git-proxy-java-dashboard/src/main/java/org/finos/gitproxy/dashboard/SecurityConfig.java index 277974e8..2c31d614 100644 --- a/git-proxy-java-dashboard/src/main/java/org/finos/gitproxy/dashboard/SecurityConfig.java +++ b/git-proxy-java-dashboard/src/main/java/org/finos/gitproxy/dashboard/SecurityConfig.java @@ -474,7 +474,13 @@ private void provisionIdpUser(Authentication auth) { if (authSource == null) return; - jdbc.upsertUser(username); + List roles = auth.getAuthorities().stream() + .map(GrantedAuthority::getAuthority) + .filter(a -> a.startsWith("ROLE_")) + .map(a -> a.substring(5)) + .toList(); + if (roles.isEmpty()) roles = List.of("USER"); + jdbc.upsertUser(username, roles); if (email != null && !email.isBlank()) { try { jdbc.upsertLockedEmail(username, email.strip().toLowerCase(), authSource);