Admin: Create admin table - #101
Conversation
34d163a to
fdeaee2
Compare
|
Graphite Automations"Request reviewers once CI passes" took an action on this PR • (09/07/26)2 reviewers were added to this PR based on Henry Chen's automation. |
| LANGUAGE sql IMMUTABLE STRICT AS | ||
| $$ SELECT LOWER(BTRIM(addr, E' \t\n\r\f\v')) $$; | ||
|
|
||
| CREATE TABLE IF NOT EXISTS "admins" ( |
There was a problem hiding this comment.
we'll need a follow up item to add everyone to this table or else we won't be able to test out the admin endpoints. do you have access to the DB?
can you also add documentation on how to write to the DB (i.e. steps for authentication, the query, etc?) so people in the future know how to add themselves?
| */ | ||
| @Repository | ||
| @RequiredArgsConstructor | ||
| public class AdminRepo { |
There was a problem hiding this comment.
if we're going to use the pattern of Repo/SqlRepo for members/matches/etc, we should also do it here. for consistency purposes.
|
|
||
| /** A {@link JdbcClient} whose single query resolves to {@code result}; the same statement mock is reused. */ | ||
| private static JdbcClient stubbedClient(final Boolean result) { | ||
| final JdbcClient jdbc = mock(JdbcClient.class); |
There was a problem hiding this comment.
the issue with these kinds of tests is that because the repo is just a wrapepr, they're not really testing anything. we're saying "here's this mock SQL repo that should return true/false for any type of query, check that the wrapper for it returns true/false".
#102 - look at what arsh is doing here with his tests. these should be testing the actual sql query functionality.
we want to test things like you already have, but also:
- fails on duplicate admin
- deleting from admin table should have the admin check be false
- inserting someone with a non-normalized email should query later as a normalized email
and so on.
| .session(session) | ||
| .with(csrf())) | ||
| .andExpect(result -> assertNotEquals( | ||
| HttpServletResponse.SC_FORBIDDEN, |
There was a problem hiding this comment.
shouldn't we just assert that it's a 2xx code? why would we only check for non forbidden?
2 participants
Admin allowlist
Adds an
adminstable soROLE_ADMINcan be granted.SecurityConfigalready gatedPOST /api/email/**andGET /api/membersbehindhasRole("ADMIN").adminsis keyed by email, with no API or UI write path — rows are inserted byhand in psql, so the app can't grant itself admin (yet).
granted authorities.
isAdminin/api/sessionis derived from those same authorities,so the SPA's view and the backend's enforcement can't disagree.
normalize_email()function (shared by the table's write trigger andAdminRepo'slookup) means casing/whitespace in a hand-typed insert doesn't matter, and the primary
key prevents casing variants coexisting as separate rows.
Adding an admin