feat: new auth manager uncomplicated #558
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
This'll also work nicely for when we move the guardians and gateways to the JWT based schemes so we just keep those in session storage instead of the passwords |
📝 Walkthrough📝 WalkthroughWalkthroughThis pull request introduces significant changes across multiple files, primarily focusing on the management of session storage and service configurations. The Changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used📓 Learnings (1)
🔇 Additional comments (12)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (11)
- apps/router/src/api/GatewayApi.ts (3 hunks)
- apps/router/src/api/GuardianApi.ts (5 hunks)
- apps/router/src/api/ServiceCheckApi.ts (0 hunks)
- apps/router/src/context/AppContext.tsx (1 hunks)
- apps/router/src/context/guardian/SetupContext.tsx (2 hunks)
- apps/router/src/context/hooks.tsx (3 hunks)
- apps/router/src/guardian-ui/utils/env.ts (1 hunks)
- apps/router/src/home/ConnectServiceModal.tsx (4 hunks)
- apps/router/src/types/gateway.tsx (1 hunks)
- apps/router/src/types/guardian.tsx (1 hunks)
- packages/ui/src/KeyValues.tsx (1 hunks)
💤 Files with no reviewable changes (1)
- apps/router/src/api/ServiceCheckApi.ts
🧰 Additional context used
📓 Learnings (3)
apps/router/src/api/GuardianApi.ts (1)
Learnt from: Kodylow PR: fedimint/ui#547 File: apps/router/src/api/GuardianApi.ts:29-35 Timestamp: 2024-10-10T21:21:37.491Z Learning: In the TypeScript file `apps/router/src/api/GuardianApi.ts`, temporary storage of the password as a class property in `GuardianApi` is acceptable during refactoring towards browser-native password management handled by the Auth Manager.apps/router/src/context/guardian/SetupContext.tsx (3)
Learnt from: Kodylow PR: fedimint/ui#547 File: apps/router/src/context/guardian/SetupContext.tsx:71-71 Timestamp: 2024-10-10T20:23:07.421Z Learning: In `apps/router/src/context/guardian/SetupContext.tsx`, the `SET_INITIAL_STATE` action in the reducer returns the current state without modification because handling is performed in the provider.Learnt from: Kodylow PR: fedimint/ui#547 File: apps/router/src/context/guardian/SetupContext.tsx:135-137 Timestamp: 2024-09-27T21:39:41.672Z Learning: In this codebase, the context is only loaded while in `guardian/id`, so `guardianId` will always be defined, and validation for `guardianId` being undefined is unnecessary.Learnt from: Kodylow PR: fedimint/ui#547 File: apps/router/src/context/hooks.tsx:51-54 Timestamp: 2024-10-10T21:28:22.154Z Learning: The function `useAppGuardianConfigs` is unused and has been removed.apps/router/src/context/hooks.tsx (2)
Learnt from: Kodylow PR: fedimint/ui#547 File: apps/router/src/context/hooks.tsx:51-54 Timestamp: 2024-10-10T21:28:22.154Z Learning: The function `useAppGuardianConfigs` is unused and has been removed.Learnt from: Kodylow PR: fedimint/ui#547 File: apps/router/src/context/hooks.tsx:69-69 Timestamp: 2024-10-10T21:30:04.372Z Learning: In `apps/router/src/context/hooks.tsx`, the function `useGuardianConfig` returns an object with `id` and `config` properties, and its return type matches the interface accordingly.
🔇 Additional comments (9)
apps/router/src/types/gateway.tsx (1)
5-7: Approve addition ofidtoGatewayConfig.The new
idproperty inGatewayConfigaligns with the PR objectives. Ensure allGatewayConfigusages are updated.apps/router/src/types/guardian.tsx (1)
4-5: Addition ofidproperty enhances guardian identification.The new
idproperty inGuardianConfigallows for unique identification of guardians, which can improve management and security.apps/router/src/context/guardian/SetupContext.tsx (1)
14-14: Approved: New import aligns with simplification goals.apps/router/src/home/ConnectServiceModal.tsx (1)
112-116: Error handling inconsistency with Enter key press.The
handleConfirmfunction is called directly on Enter key press, bypassing the error state reset that occurs in the onClick handler of the confirm button. This could lead to stale error messages.apps/router/src/context/AppContext.tsx (1)
188-198: Changes look good.The addition of the
idproperty and the use ofnewServicein checks and payloads improve service identification and consistency.apps/router/src/api/GuardianApi.ts (1)
310-310: Potential security issue with null authenticationUsing
nullfor authentication when no password is available might lead to unintended authentication behavior. Consider using an empty string or omitting theauthfield entirely when no password is present.apps/router/src/context/hooks.tsx (3)
Line range hint
84-134: Potential performance issue with dependency arrayIncluding
idin theuseEffectdependency array might cause unnecessary re-renders ifidchanges frequently.
442-445: Potential security risk with session storageSetting
state.needsAuthtofalsebased on the presence ofidinsessionStoragemight introduce a security vulnerability if not properly managed.
Line range hint
447-502: Potential performance issue with dependency arrayIncluding
apianddispatchin theuseEffectdependency array might cause unnecessary re-renders if these values change frequently.
Kodylow
changed the title
Okay entire new auth context was getting overly complicated and there's some pretty serious crypto concerns, so made this far simpler one that just uses the browser's password manager, saving passwords per service in the password manager and only ever holding in session storage after you've logged in for the session.
Summary by CodeRabbit
Release Notes
New Features
idproperty for session management in Gateway and Guardian APIs.setSessionPasswordmethod in the Guardian API for improved password handling.Logincomponent with aserviceIdprop for better accessibility.Bug Fixes
Refactor
Type Updates
GatewayConfigandGuardianConfigtypes to include anidproperty.