Skip to content

Commit

Permalink
Add OAuth tests (#2874)
Browse files Browse the repository at this point in the history
* Improve error message when using invalid client_id during code exchange

* Extract SPA example OAuth client in own package

* wip

* remove dependency on get-port

* Properly configure jest to only transpile "get-port" from node_modules

https://jestjs.io/docs/configuration#transformignorepatterns-arraystring

* Use dynamically assigned port number during tests

* use puppeteer to run tests

* remove login input "id" attribute

* code style

* add missing declaration

* tidy

* headless

* remove get-port dependency

* fix tests/proxied/admin.test.ts

* fix tests

* Allow unsecure oauth providers through configuration

* transpile "lande" during ozone tests

* Cache Puppeteer browser binaries

* Use puppeteer cache during all workflow steps

* remove use of set-output

* use get-port in xrpc-server tests

* Renamed to allowHttp

* tidy

* tidy
  • Loading branch information
matthieusieben authored Oct 18, 2024
1 parent 60df3fc commit 7f26b17
Show file tree
Hide file tree
Showing 69 changed files with 1,247 additions and 419 deletions.
7 changes: 7 additions & 0 deletions .changeset/clever-walls-fold.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@atproto/oauth-client-browser": patch
"@atproto/oauth-client": patch
---

Add `allowHttp` OAuthClient construction option to allow working with "http:" oauth providers (for development & testing purposes).

5 changes: 5 additions & 0 deletions .changeset/eleven-avocados-switch.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@atproto/oauth-types": minor
---

Allow oauthIssuerIdentifier to be an "http:" url. Make sure to manually check for "http:" issuers if you don't allow them.
5 changes: 5 additions & 0 deletions .changeset/gentle-socks-punch.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@atproto-labs/did-resolver": patch
---

Add "allowHttp" did:web method option
5 changes: 5 additions & 0 deletions .changeset/hungry-ways-decide.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@atproto/oauth-client-node": patch
---

Bugfix: Prevent accidental override of `NodeOAuthClient` constructor options
5 changes: 5 additions & 0 deletions .changeset/thirty-jeans-itch.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@atproto/oauth-types": minor
---

Remove ALLOW_UNSECURE_ORIGINS constant
5 changes: 5 additions & 0 deletions .changeset/witty-masks-wave.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@atproto/oauth-provider": patch
---

Improve error message when invalid client id used during code exchange
5 changes: 5 additions & 0 deletions .changeset/young-paws-poke.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@atproto/oauth-types": patch
---

Improve typing of oauthIssuerIdentifierSchema
21 changes: 21 additions & 0 deletions .github/workflows/repo.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,13 @@ jobs:
with:
node-version: 18
cache: 'pnpm'
- name: Get current month
run: echo "CURRENT_MONTH=$(date +'%Y-%m')" >> $GITHUB_ENV
- uses: actions/cache@v4
name: Cache Puppeteer browser binaries
with:
path: ~/.cache
key: ${{ env.CURRENT_MONTH }}-${{ runner.os }}
- run: pnpm i --frozen-lockfile
- run: pnpm build
- uses: actions/upload-artifact@v4
Expand All @@ -49,6 +56,13 @@ jobs:
with:
node-version: 18
cache: 'pnpm'
- name: Get current month
run: echo "CURRENT_MONTH=$(date +'%Y-%m')" >> $GITHUB_ENV
- uses: actions/cache@v4
name: Cache Puppeteer browser binaries
with:
path: ~/.cache
key: ${{ env.CURRENT_MONTH }}-${{ runner.os }}
- run: pnpm i --frozen-lockfile
- uses: actions/download-artifact@v4
with:
Expand All @@ -69,6 +83,13 @@ jobs:
with:
node-version: 18
cache: 'pnpm'
- name: Get current month
run: echo "CURRENT_MONTH=$(date +'%Y-%m')" >> $GITHUB_ENV
- uses: actions/cache@v4
name: Cache Puppeteer browser binaries
with:
path: ~/.cache
key: ${{ env.CURRENT_MONTH }}-${{ runner.os }}
- run: pnpm i --frozen-lockfile
- uses: actions/download-artifact@v4
with:
Expand Down
3 changes: 1 addition & 2 deletions packages/api/jest.config.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
/** @type {import('jest').Config} */
module.exports = {
displayName: 'API',
transform: { '^.+\\.(t|j)s$': '@swc/jest' },
transformIgnorePatterns: [`<rootDir>/node_modules/(?!get-port)`],
transform: { '^.+\\.ts$': '@swc/jest' },
testTimeout: 60000,
setupFiles: ['<rootDir>/../../jest.setup.ts'],
setupFilesAfterEnv: ['<rootDir>/jest.setup.ts'],
Expand Down
1 change: 0 additions & 1 deletion packages/api/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
},
"devDependencies": {
"@atproto/lex-cli": "workspace:^",
"get-port": "^6.1.2",
"jest": "^28.1.2",
"prettier": "^3.2.5",
"typescript": "^5.6.2"
Expand Down
14 changes: 7 additions & 7 deletions packages/api/tests/dispatcher.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { AddressInfo } from 'node:net'
import assert from 'assert'
import getPort from 'get-port'
import {
AtpAgent,
AtpSessionEvent,
Expand Down Expand Up @@ -443,8 +443,8 @@ describe('AtpAgent', () => {

describe('App labelers header', () => {
it('adds the labelers header as expected', async () => {
const port = await getPort()
const server = await createHeaderEchoServer(port)
const server = await createHeaderEchoServer()
const port = (server.address() as AddressInfo).port
const agent = new AtpAgent({ service: `http://localhost:${port}` })
const agent2 = new AtpAgent({ service: `http://localhost:${port}` })

Expand All @@ -470,8 +470,8 @@ describe('AtpAgent', () => {

describe('configureLabelers', () => {
it('adds the labelers header as expected', async () => {
const port = await getPort()
const server = await createHeaderEchoServer(port)
const server = await createHeaderEchoServer()
const port = (server.address() as AddressInfo).port
const agent = new AtpAgent({ service: `http://localhost:${port}` })

agent.configureLabelers(['did:plc:test1'])
Expand All @@ -492,8 +492,8 @@ describe('AtpAgent', () => {

describe('configureProxy', () => {
it('adds the proxy header as expected', async () => {
const port = await getPort()
const server = await createHeaderEchoServer(port)
const server = await createHeaderEchoServer()
const port = (server.address() as AddressInfo).port
const agent = new AtpAgent({ service: `http://localhost:${port}` })

const res1 = await agent.com.atproto.server.describeServer()
Expand Down
36 changes: 18 additions & 18 deletions packages/api/tests/util/echo-server.ts
Original file line number Diff line number Diff line change
@@ -1,21 +1,21 @@
import http from 'node:http'
import { once } from 'node:events'
import { createServer } from 'node:http'

export async function createHeaderEchoServer(port: number) {
return new Promise<http.Server>((resolve) => {
const server = http.createServer()

server
.on('request', (request, response) => {
response.setHeader('content-type', 'application/json')
response.end(
JSON.stringify({
...request.headers,
did: 'did:web:fake.com',
availableUserDomains: [],
}),
)
})
.on('listening', () => resolve(server))
.listen(port)
export async function createHeaderEchoServer(port: number = 0) {
const server = createServer((req, res) => {
res.writeHead(200, undefined, { 'content-type': 'application/json' })
res.end(
JSON.stringify({
...req.headers,
did: 'did:web:fake.com',
availableUserDomains: [],
}),
)
})

server.listen(port)

await once(server, 'listening')

return server
}
2 changes: 1 addition & 1 deletion packages/bsky/jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
module.exports = {
displayName: 'Bsky App View',
transform: { '^.+\\.(t|j)s$': '@swc/jest' },
transformIgnorePatterns: [`<rootDir>/node_modules/(?!get-port)`],
transformIgnorePatterns: ['/node_modules/.pnpm/(?!(get-port)@)'],
testTimeout: 60000,
setupFiles: ['<rootDir>/../../jest.setup.ts'],
}
2 changes: 1 addition & 1 deletion packages/identity/jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@
module.exports = {
displayName: 'Identity',
transform: { '^.+\\.(t|j)s$': '@swc/jest' },
transformIgnorePatterns: [`<rootDir>/node_modules/(?!get-port)`],
transformIgnorePatterns: ['/node_modules/.pnpm/(?!(get-port)@)'],
setupFiles: ['<rootDir>/../../jest.setup.ts'],
}
15 changes: 14 additions & 1 deletion packages/internal/did-resolver/src/methods/web.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,31 @@ const fetchSuccessHandler = pipe(

export type DidWebMethodOptions = {
fetch?: Fetch
/** @default true */
allowHttp?: boolean
}

export class DidWebMethod implements DidMethod<'web'> {
protected readonly fetch: Fetch<unknown>
protected readonly allowHttp: boolean

constructor({ fetch = globalThis.fetch }: DidWebMethodOptions = {}) {
constructor({
fetch = globalThis.fetch,
allowHttp = true,
}: DidWebMethodOptions = {}) {
this.fetch = bindFetch(fetch)
this.allowHttp = allowHttp
}

async resolve(did: Did<'web'>, options?: ResolveDidOptions) {
const didDocumentUrl = buildDidWebDocumentUrl(did)

if (!this.allowHttp && didDocumentUrl.protocol === 'http:') {
throw new Error(
`Cannot resolve DID document for localhost: ${didDocumentUrl}`,
)
}

// Note we do not explicitly check for "localhost" here. Instead, we rely on
// the injected 'fetch' function to handle the URL. If the URL is
// "localhost", or resolves to a private IP address, the fetch function is
Expand Down
3 changes: 1 addition & 2 deletions packages/lexicon/jest.config.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
/** @type {import('jest').Config} */
module.exports = {
displayName: 'Lexicon',
transform: { '^.+\\.(t|j)s$': '@swc/jest' },
transformIgnorePatterns: [`<rootDir>/node_modules/(?!get-port)`],
transform: { '^.+\\.ts$': '@swc/jest' },
setupFiles: ['<rootDir>/../../jest.setup.ts'],
}
60 changes: 60 additions & 0 deletions packages/oauth/oauth-client-browser-example/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
{
"name": "@atproto/oauth-client-browser-example",
"version": "0.0.0",
"license": "MIT",
"description": "Example single page application app using ATPROTO OAuth",
"keywords": [
"example",
"spa",
"atproto",
"oauth",
"browser",
"client"
],
"homepage": "https://atproto.com",
"repository": {
"type": "git",
"url": "https://github.com/bluesky-social/atproto",
"directory": "packages/oauth/oauth-client-browser"
},
"type": "commonjs",
"exports": {
".": {
"default": "./dist/files.json"
}
},
"files": [
"dist"
],
"dependencies": {},
"devDependencies": {
"@atproto-labs/rollup-plugin-bundle-manifest": "workspace:*",
"@atproto/api": "workspace:*",
"@atproto/oauth-client": "workspace:*",
"@atproto/oauth-client-browser": "workspace:*",
"@atproto/oauth-types": "workspace:*",
"@atproto/xrpc": "workspace:*",
"@rollup/plugin-commonjs": "^25.0.7",
"@rollup/plugin-json": "^6.1.0",
"@rollup/plugin-html": "^1.0.4",
"@rollup/plugin-node-resolve": "^15.2.3",
"@rollup/plugin-replace": "^5.0.5",
"@rollup/plugin-terser": "^0.4.4",
"@rollup/plugin-typescript": "^11.1.6",
"@types/react": "^18.2.50",
"@types/react-dom": "^18.2.18",
"autoprefixer": "^10.4.17",
"postcss": "^8.4.33",
"react": "^18.2.0",
"react-dom": "^18.2.0",
"rollup": "^4.13.0",
"rollup-plugin-postcss": "^4.0.2",
"rollup-plugin-serve": "^1.1.1",
"tailwindcss": "^3.4.1",
"typescript": "^5.6.3"
},
"scripts": {
"build": "rollup --config rollup.config.js",
"dev": "rollup --config rollup.config.js --watch"
}
}
Loading

0 comments on commit 7f26b17

Please sign in to comment.