Repository navigation
Conversation
There was a problem hiding this comment.
Copilot was unable to review this pull request because there is no eligible user to bill.
An enterprise owner can allow the organization to be billed for reviews like this by enabling Copilot code review for users without a license.
Add encrypted database and read-only environment credential providers for one GitHub installation, transactional identity repositories, and their storage contracts. Co-authored-by: Ben Thomas <ben.thomas@microsoft.com> Co-authored-by: Nishiki <hello@nshki.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
4924f22 to
0473854
Compare
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
The read interface exposes mutations outside locked transactions, allowing callers to bypass serialization and leave partial updates when an operation fails.
6 open issues
Restrict read API to queries and lock mutations · New Assert session deletion before transaction rollback · New Ensure the lock test waits for the blocked transaction · New Preserve validation errors after credential decryption · New Remove unsupported authorization-model promise · New Document audit retention and export requirements · New
Review effort: Max
|
|
||
| export function createIdentityStore(client: PrismaClient): IdentityStore { | ||
| return { | ||
| read: createIdentityRecords(client), |
| store.lock(async (records) => { | ||
| await records.saveGitHubIntegrationCredentials("replacement-encrypted-fixture") | ||
| await records.advanceGitHubIntegrationGeneration() | ||
| await records.deleteSessions() |
| const { store } = await database() | ||
| await store.lock(() => Promise.resolve()) | ||
| let entered!: () => void | ||
| let release!: () => void | ||
| const started = new Promise<void>((resolve) => { | ||
| entered = resolve | ||
| }) | ||
| const gate = new Promise<void>((resolve) => { | ||
| release = resolve | ||
| }) | ||
| const first = store.lock(async (records) => { | ||
| await records.advanceGitHubIntegrationGeneration() | ||
| entered() | ||
| await gate | ||
| }) | ||
| await started | ||
| const second = store.lock(async (records) => { | ||
| const state = await records.githubIntegrationState() | ||
| await records.advanceGitHubIntegrationGeneration() | ||
| return state.generation | ||
| }) | ||
| release() |
| try { | ||
| const data = Buffer.from(encryptedCredentials, "base64") | ||
| const decipher = createDecipheriv("aes-256-gcm", key, data.subarray(0, 12)) | ||
| decipher.setAAD(credentialContext) | ||
| decipher.setAuthTag(data.subarray(12, 28)) | ||
| const plaintext = Buffer.concat([decipher.update(data.subarray(28)), decipher.final()]) | ||
| return validateGitHubCredentials(JSON.parse(plaintext.toString("utf8"))) | ||
| } catch { | ||
| throw new Error("Cannot decrypt GitHub App credentials. Restore the configured encryption key.") | ||
| } |
| @@ -0,0 +1,34 @@ | |||
| # Hub identity | |||
|
|
|||
| Hub persists a provider-neutral identity foundation. User authentication proves an external identity. Hub installation membership decides whether that user belongs to this Hub and whether they are an administrator. Later authorization work will let Hub own prototype access independently of repository permissions. | |||
| - `AuthenticationProvider` and its versioned revisions store provider-neutral authentication configuration. An OIDC-only configuration does not require GitHub source-integration credentials. | ||
| - `Session` records the Hub user, external identity, and authentication-provider revision that established it. GitHub source-integration credential rotation does not change session identity context. | ||
| - `AuthorizationAttempt` stores digested state and browser bindings, a PKCE verifier, an optional OIDC nonce, provider revision, purpose, optional originating session, and expiry. Consumption deletes the record in the same transaction so concurrent callbacks cannot reuse it. | ||
| - `SecurityAudit` stores the event and minimal optional identity context. Audit records do not create the future prototype authorization model. |
There was a problem hiding this comment.
Nit: the added documentation here has AI writing artifacts like a bunch of hyphenated phrases. Could be worth running it all through the repo's documentation conventions to see if that produces clearer and more concise writing.
|
|
||
| Tests must release resources they create, such as servers, files, sockets, and event listeners. Use `try/finally` or register disposal with Vitest's `onTestFinished` when creating the resource. Clearing the module cache only affects subsequent imports. Reset any deliberately shared fixture state before each case. | ||
|
|
||
| Keep shared server test helpers under the owning function's `test/` directory. TypeScript checks them, but the API build excludes them. |
There was a problem hiding this comment.
| Keep shared server test helpers under the owning function's `test/` directory. TypeScript checks them, but the API build excludes them. | |
| Keep project-wide test helpers in the appropriate `test/` directory and function-specific helpers beside their owning function. TypeScript checks both, but the API build excludes them. |
For better clarity.
| @@map("authentication_providers") | ||
| } | ||
|
|
||
| model AuthenticationProviderRevision { |
There was a problem hiding this comment.
We should break individual model definitions into their own files so we don't overload any one Prisma file.
There was a problem hiding this comment.
Just by the model name alone, it's unclear as to what this is. We should consider renaming to something like GitHubAppInstallation to make it clearer what this is.
| id String @id @db.Uuid | ||
|
|
||
| // Attribute columns | ||
| generation Int @default(0) |
There was a problem hiding this comment.
generation is currently an incremental counter that seems to only get bumped in tests. If there is a way to refactor it out completely, that would be ideal since there is no production use case for this column.



Why
Hub needs a durable identity foundation that does not make one sign-in provider or GitHub integration the definition of a user. This layer gives later authentication and authorization work stable records while keeping provider-specific behavior outside the core identity model. Refs #116.
What changes
Hub now persists users, external identities, installation membership, versioned authentication-provider configuration, sessions, one-use authorization attempts, and security audit records. Repository and domain interfaces keep those records provider-neutral and support transactional updates under a shared store lock.
GitHub source access remains a separate optional integration. Operators can use encrypted database credentials or complete read-only environment settings without coupling GitHub credentials to user identity or session context.
The change also retains automatic redacted request logging from the current Hub baseline. It adds persistence only: there are no sign-in routes, product authorization rules, membership synchronization, or UI changes in this layer.