-
Notifications
You must be signed in to change notification settings - Fork 8
494 Member Application System #502
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: zami-nobla
Are you sure you want to change the base?
Conversation
…aceholder contact email/phone
✅ Deploy Preview for volunteer-bitsofgood ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
* added menu button on home page for mobile use, adjust styling for auth page and landing page * Format --------- Co-authored-by: Hayden Carpenter <[email protected]>
|
Approval flow is broken/missing.
|
|
#518 should be complete now |
|
#519 should be complete now |
|
#520 should(?) also be complete now |
|
Confer with @Indy1131 about admin settings backend + approval dashboard |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Disclaimer: This comment was AI-generated and is not necessarily completely accurate. Please take code comments with a grain of salt.
Greptile Overview
Greptile Summary
Implements a member application approval system where users can register and await admin approval before accessing the platform. The implementation adds application status tracking (pending, approved, rejected) to the User model, creates frontend components to display application status, and integrates access control throughout the application.
Key Changes:
- Database schema updated with
applicationStatus,appliedAt,approvedAt,approvedBy,rejectionReason, andrejectedAtfields - Organizations can now enable
requiresUserApprovalflag and configure custom registration forms - User creation flow checks organization approval settings and sets appropriate application status
ApplicationStatusWrappercomponent enforces access control by routing non-approved users to status pages- Migration script sets existing users to "approved" status
- New
UserRegistrationResponsemodel stores registration form responses separately
Critical Issues Found:
- Google OAuth user creation bypasses the organization's approval workflow entirely - users signing up via Google will always get "approved" status by schema default, regardless of the organization's
requiresUserApprovalsetting - Migration script doesn't set
approvedAttimestamp when marking existing users as approved, creating data inconsistency
Confidence Score: 2/5
- This PR has critical logical flaws that allow users to bypass the approval workflow via Google OAuth
- The Google OAuth flow completely bypasses organization approval requirements, allowing any user to gain instant access by signing in with Google even when the organization requires application approval. This undermines the entire feature. Additionally, the migration creates inconsistent data by not setting approvedAt timestamps. The core approval workflow logic for credential-based registration is well-implemented with proper rollback handling, but the OAuth bypass is a security/access control issue that needs resolution.
src/pages/api/auth/[...nextauth].tsrequires immediate attention to enforce approval workflow for Google OAuth users, andserver/mongodb/migrations/add-application-status.jsneeds to set approvedAt timestamps
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| server/mongodb/migrations/add-application-status.js | 3/5 | Migration script sets applicationStatus to "approved" for existing users but omits approvedAt timestamp, creating data inconsistency |
| src/pages/api/auth/[...nextauth].ts | 2/5 | Google OAuth user creation bypasses organization approval workflow - applicationStatus not set, defaults to approved regardless of org settings |
| server/actions/users_new.ts | 4/5 | User creation logic properly implements application approval workflow with rollback handling for registration responses |
| server/mongodb/models/User/index.ts | 5/5 | Added application status fields (applicationStatus, appliedAt, approvedAt, approvedBy, rejectionReason, rejectedAt) with proper schema configuration |
| src/screens/ApplicationStatus/ApplicationStatusWrapper.jsx | 5/5 | Access control wrapper correctly routes users based on applicationStatus to pending/rejected pages or main app |
Sequence Diagram
sequenceDiagram
participant User
participant AuthForm
participant API as /api/users
participant UserAction as createUserFromCredentials
participant Organization
participant UserModel as User Model
participant RegResponse as UserRegistrationResponse
participant NextAuth
participant AppWrapper as ApplicationStatusWrapper
participant PendingPage
participant MainApp
Note over User,MainApp: New User Registration Flow
User->>AuthForm: Submit registration form
AuthForm->>API: POST /api/users (email, password, orgCode, registrationFormResponses)
API->>UserAction: createUserFromCredentials(userData)
UserAction->>UserModel: Check if email exists
UserModel-->>UserAction: Email available
UserAction->>Organization: Find by slug (orgCode)
Organization-->>UserAction: Organization found
alt User is invited admin
UserAction->>UserAction: Set role=admin, applicationStatus=approved
else Organization requires approval
UserAction->>UserAction: Set applicationStatus=pending
else No approval required
UserAction->>UserAction: Set applicationStatus=approved
end
UserAction->>UserModel: Create user with applicationStatus
UserModel-->>UserAction: User created
alt Organization requires approval
UserAction->>RegResponse: Save registration form responses
RegResponse-->>UserAction: Responses saved
Note over UserAction,RegResponse: Rollback user if this fails
end
UserAction-->>API: Return user and status
API-->>AuthForm: Success response
AuthForm->>NextAuth: signIn with credentials
Note over User,MainApp: Login and Access Control Flow
User->>NextAuth: Access application
NextAuth->>UserModel: Fetch user data
UserModel-->>NextAuth: User with applicationStatus
NextAuth->>AppWrapper: Render with session data
alt applicationStatus == "pending"
AppWrapper->>PendingPage: Show pending status
PendingPage-->>User: "Application Under Review" message
else applicationStatus == "rejected"
AppWrapper->>User: Show rejection page with reason
else applicationStatus == "approved"
AppWrapper->>MainApp: Render main application
MainApp-->>User: Full platform access
end
Additional Comments (1)
-
src/pages/api/auth/[...nextauth].ts, line 101-111 (link)logic: Google OAuth users created here won't have
applicationStatusset, causing them to default to "approved" via schema defaults. However, this bypasses the organization'srequiresUserApprovalsetting - Google sign-up users will get instant access even when the org requires approval workflow.
28 files reviewed, 2 comments
| const res = await db | ||
| .collection("users") | ||
| .updateMany( | ||
| { applicationStatus: { $exists: false } }, | ||
| { $set: { applicationStatus: "approved" } } | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: Migration sets applicationStatus: "approved" for existing users but doesn't set approvedAt timestamp. This creates data inconsistency - approved users should have an approvedAt timestamp per the schema design in User model.
| const res = await db | |
| .collection("users") | |
| .updateMany( | |
| { applicationStatus: { $exists: false } }, | |
| { $set: { applicationStatus: "approved" } } | |
| ); | |
| .updateMany( | |
| { applicationStatus: { $exists: false } }, | |
| { $set: { applicationStatus: "approved", approvedAt: new Date() } } | |
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i agree this should be addressed
|
Greptile asks about const user_data = {
_id,
imageUrl: message.user.image,
firstName: message.user.name.split(" ")[0] ?? "first",
lastName: message.user.name.split(" ")[1] ?? "last",
phone: "",
email: message.user.email,
};
const user = new User(user_data);
await user.save();In [...nextauth].ts, which does indeed not set the proper status when using oauth. |
…ication status assignment
|
Updated Google auth flow by setting application status after org code update from updateOrganizationId in organization update modal |
Member Application System
Implements desired member application system
Closes #494
Dev Checklist