Skip to content
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

Feat: auth middleware #90

Merged
merged 70 commits into from
Mar 19, 2025
Merged

Feat: auth middleware #90

merged 70 commits into from
Mar 19, 2025

Conversation

kbventures
Copy link
Contributor

@kbventures kbventures commented Feb 26, 2025

Summary

APIs \ Auth Login Access level
Auth
POST /auth/sign-up - -
GET /auth/activate - -
POST /auth/sign-in - -
Profiles
GET /profiles - -
GET /profiles/{id} - -
POST /profiles O organizer
PATCH /profiles/{id} O admin or volunteer with own id
DELETE /profiles/{id} O admin
Teams
GET /teams O volunteer
GET /teams/{id} O volunteer
POST /teams O admin
PATCH /teams/{id} O organizer
DELETE /teams/{id} O admin

Tests

Additional information

Implement KV store checks
and Add session expiry validation
Implement KV store checks
and Add session expiry validation
@kbventures kbventures self-assigned this Feb 26, 2025
@kbventures
Copy link
Contributor Author

Role-based service VS Seperate Routes

| Role-Based Service | - Centralized logic
- Easy to modify rules
- Good for complex permissions | - More initial setup
- More complex structure |

| Separate Routes | - Clear middleware chain
- Reusable role checks
- Easy to add new role-based routes | - More files to manage
- Need to maintain middleware separately |

The middleware approach (Option 2) is particularly powerful because:
Middleware is reusable across different routes
Role checks are separated from business logic
Easy to add new role-based routes
Clear separation of concerns

There are other options but I didn't really think single route was a good idea as we'd probably end up having to refactor it later.

@lylaminju lylaminju requested a review from madcampos March 10, 2025 17:02
@lylaminju lylaminju marked this pull request as ready for review March 12, 2025 15:44
@lylaminju lylaminju self-assigned this Mar 14, 2025
@torontojs torontojs deleted a comment from madcampos Mar 14, 2025
madcampos
madcampos previously approved these changes Mar 18, 2025
Copy link
Collaborator

@madcampos madcampos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 threads left to resolve, otherwise looks good to me.

NOTE: All of these are non-blocking and/or require more discussion, so I'm approving this PR for us to move forward and resolve those issues separately.

@kbventures kbventures requested a review from madcampos March 19, 2025 17:47
@kbventures kbventures merged commit 45f5261 into dev Mar 19, 2025
2 checks passed
@kbventures kbventures deleted the auth-middleware branch March 19, 2025 18:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants