OAuth: 2FA #2633
OAuth: 2FA #2633
Conversation
matthieusieben
force-pushed
the
feat-oauth-entryway
branch
from
252f075
to
6cb51a0
Compare
fix(oauth): support building from parcel feat(oauth): add runtime lock support to prevent concurrent session updates feat(oauth): improve metadata validation fix(oauth): allow use of handle as login hint fix: proper parsing of authorization header feat(oauth): add email 2fa support feat(oauth): adapt auth UI to match app UI
matthieusieben
force-pushed
the
feat-oauth-entryway
branch
from
6cb51a0
to
07973ec
Compare
| return `SHA-${name.slice(-3)}` | ||
| default: | ||
| throw new TypeError(`Unknown hash algorithm ${name}`) | ||
| const buf = await crypto.subtle.digest(`SHA-${name.slice(3)}`, data) |
There was a problem hiding this comment.
I know DigestAlgorithm describes SHAs, but it seems like it would be possible for a new digest algo to be introduced to the type without triggering any issue here. Could we ensure that it begins with "sha" using types (maybe satisfies?) or a runtime assertion?
There was a problem hiding this comment.
not sure why I didn't keep the check... i'll add it back
| // actions={ | ||
| // onOther && ( | ||
| // <Button onClick={onOther} aria-label={otherAria}> | ||
| // {otherLabel} | ||
| // <CaretRight className="h-4 ml-2" /> | ||
| // </Button> | ||
| // ) | ||
| // } |
There was a problem hiding this comment.
Can we remove this dead code?
| className, | ||
| ...props | ||
| }: InputHTMLAttributes<HTMLInputElement>) { | ||
| const [id] = useState(generateUniqueId) |
There was a problem hiding this comment.
If props contains id then the input and label could get out of sync with each other. Perhaps the unique id should be used as a fallback.
| switch (desc) { | ||
| case 'Unknown request_uri': // Request was removed from database | ||
| case 'This request has expired': | ||
| switch (true) { | ||
| case desc?.startsWith('Unknown request_uri'): // Request was removed from database | ||
| case desc === 'This request has expired': | ||
| return 'This sign-in session has expired' | ||
| default: | ||
| return desc || 'An unknown error occurred' |
There was a problem hiding this comment.
Just a small style nit, but I find it quite a bit more common to see a regular old if statement for this kind of thing.
if (
desc?.startsWith('Unknown request_uri') ||
desc === 'This request has expired'
) {
// ...
} else {
// ...
}| @@ -75,13 +62,15 @@ function parseColor(color: string): undefined | ParsedColor { | |||
| return undefined | |||
| } | |||
|
|
|||
| const rgbMatch = color.match(/rgb\((\d+),\s*(\d+),\s*(\d+)\)/) | |||
| const rgbMatch = color.match(/rgb\(\s*(\d+)\s*,\s*(\d+)\s*,\s*(\d+)\s*\)/) | |||
There was a problem hiding this comment.
Do we want some ^/$ in here? (And same question below for rgba.)
There was a problem hiding this comment.
Made the whole customization more strongly parsed & pre-computed during init of the lib
| const requestUri = authorizeUrl.searchParams.get('request_uri') | ||
| if (!requestUri) return | ||
|
|
||
| // @TODO find a way to actually do this |
There was a problem hiding this comment.
Just flagging to ensure this TODO is still intended to be here.
There was a problem hiding this comment.
This is fine for now. Improved my comments there.
| const response = await fetch(request) | ||
|
|
||
| if (response.status !== 200) { | ||
| throw new TypeError(`Failed to fetch client metadata: ${response.status}`) | ||
| } | ||
|
|
||
| const mime = response.headers.get('content-type')?.split(';')[0].trim() | ||
| if (mime !== 'application/json') { | ||
| throw new TypeError(`Invalid client metadata content type: ${mime}`) | ||
| } | ||
|
|
||
| const json: unknown = await response.json() |
There was a problem hiding this comment.
In the case of an error it looks like the response body would not be read. If I'm not mistaken, in node environments the body should always be read to avoid a leak.
Invalid oauth customization would cause the server to crash at startup.
Co-authored-by: devin ivy <devinivy@gmail.com>
fix(pds): show user did in logs fix(ozone): show user did in logs
…cements * origin/main: (181 commits) Minor OAuth client fixes (#2640) Version packages (#2639) OAuth: 2FA (#2633) Version packages (#2622) Update data source for `getSuggestedFollowsByActor` (#2630) Add in-memory did cache to Ozone backend (#2635) Filter out reference lists from `getLists` (#2629) lexicons: add missing ozone Tag event type to unions (#2632) ✨ Add ozone proxy for getLikes and getRepostedBy (#2624) Upgrade pnpm/action-setup in workflows (#2625) ✨ Add proxy for user typeahead through ozone (#2612) Fix development commands (#2623) Add starter packs to post hydration (#2613) Social proof blocks (#2603) Appview: apply hosting status in getRecord (#2620) Add `labelersPref` to `getPreferences` union return types (#2554) Version packages (#2618) Add new preference and api for bsky app state; also put preference updates within transactional lock regions (#2492) remove mentions of sandbox (#2611) Appview: simple fix for no-hosted known followers (#2609) ...
This PR contains the non-SDK changes of
feat-oauth-client(#2483)