-
Notifications
You must be signed in to change notification settings - Fork 1
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
Migrate from role-based to permission-based auth #336
base: main
Are you sure you want to change the base?
Conversation
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.
Copilot reviewed 25 out of 40 changed files in this pull request and generated 1 comment.
Files not reviewed (15)
- backend/.sqlc/migrations/20241215194302_initial_schema.sql: Language not supported
- backend/.sqlc/queries/projects.sql: Language not supported
- backend/.sqlc/queries/transactions.sql: Language not supported
- backend/.sqlc/queries/users.sql: Language not supported
- backend/internal/jwt/generate.go: Evaluated as low risk
- backend/internal/middleware/types.go: Evaluated as low risk
- backend/internal/tests/company_test.go: Evaluated as low risk
- backend/internal/tests/auth_test.go: Evaluated as low risk
- backend/db/users.sql.go: Evaluated as low risk
- backend/db/models.go: Evaluated as low risk
- backend/db/transactions.sql.go: Evaluated as low risk
- backend/internal/tests/jwt_middleware_test.go: Evaluated as low risk
- backend/internal/tests/helpers.go: Evaluated as low risk
- backend/internal/jwt/types.go: Evaluated as low risk
- backend/db/projects.sql.go: Evaluated as low risk
Comments suppressed due to low confidence (3)
backend/internal/middleware/jwt.go:38
- The request method should be checked before allowing access based on the PermViewAllProjects permission to ensure that only GET requests are allowed without further validation.
if c.Request().Method == http.MethodGet {
backend/internal/tests/comments_test.go:21
- The struct name 'CommentResponse' is appropriate and consistent with its functionality.
type CommentResponse struct {
backend/internal/tests/comments_test.go:32
- The function name 'TestCommentEndpoints' is appropriate and consistent with its functionality.
func TestCommentEndpoints(t *testing.T) {
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.
Copilot reviewed 25 out of 40 changed files in this pull request and generated 1 comment.
Files not reviewed (15)
- backend/.sqlc/migrations/20241215194302_initial_schema.sql: Language not supported
- backend/.sqlc/queries/projects.sql: Language not supported
- backend/.sqlc/queries/transactions.sql: Language not supported
- backend/.sqlc/queries/users.sql: Language not supported
- backend/internal/permissions/permissions.go: Evaluated as low risk
- backend/db/models.go: Evaluated as low risk
- backend/internal/jwt/generate.go: Evaluated as low risk
- backend/internal/middleware/types.go: Evaluated as low risk
- backend/internal/tests/company_test.go: Evaluated as low risk
- backend/internal/tests/auth_test.go: Evaluated as low risk
- backend/db/users.sql.go: Evaluated as low risk
- backend/db/transactions.sql.go: Evaluated as low risk
- backend/internal/tests/jwt_middleware_test.go: Evaluated as low risk
- backend/internal/jwt/types.go: Evaluated as low risk
- backend/internal/tests/helpers.go: Evaluated as low risk
Comments suppressed due to low confidence (3)
backend/internal/middleware/request_validator.go:209
- [nitpick] The error message for 'valid_permissions' could be more descriptive. Consider providing more context or examples of valid permissions.
return fmt.Sprintf("%s contains invalid permissions", field)
backend/db/projects.sql.go:321
- The comment is unclear. It should be more descriptive to explain the purpose of the permission check, such as 'Check if the user has the PermViewAllProjects permission'.
-- Check for PermViewAllProjects (1 << 0)
backend/internal/tests/helpers_test.go:173
- [nitpick] The use of fmt.Sprint for permissions in the JSON string is unconventional. Consider using a more straightforward approach to set the permissions value.
"permissions": ` + fmt.Sprint(permissions.PermStartupOwner) + `,
Description
Refactored backend authorization system from role-based to permission-based access control. This change provides more specific control over user permissions and better scalability for future features.
Linked Issues
Testing
/internal/tests/auth_test.go
/internal/tests/companies_test.go
/internal/tests/projects_test.go
/internal/tests/teams_test.go
/internal/tests/transactions_test.go
Checklist
Before opening this PR, make sure the PR:
Additionally, make sure that: