[KLO-238] Fix/auth login for non verified users#298
Conversation
- deletes session, and cookie when logout happens
nxtcoder17
changed the title
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @nxtcoder17 - I've reviewed your changes and they look great!
General suggestions:
- Ensure that the new permission model introduced by the changes aligns with the intended security posture.
- Review the migration of domain logic to the entities package for completeness and correctness.
- Verify that the new error handling patterns are consistently applied across the codebase.
- Double-check for any unintended duplications or omissions in the GraphQL schema updates.
Here's what I looked at during the review
- 🟡 General issues: 3 issues found
- 🟡 Security: 1 issue found
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Docstrings: all looks good
Thanks for using Sourcery. We offer it for free for open source projects and would be very grateful if you could help us grow. If you like it, would you consider sharing Sourcery on your favourite social media? ✨
| // for clusters | ||
| t.CreateCluster: []t.Role{t.RoleAccountOwner, t.RoleAccountAdmin, t.RoleAccountMember}, | ||
| t.DeleteCluster: []t.Role{t.RoleAccountOwner, t.RoleAccountAdmin, t.RoleAccountMember}, | ||
| t.UpdateCluster: []t.Role{t.RoleAccountOwner, t.RoleAccountAdmin}, |
There was a problem hiding this comment.
🚨 suggestion (security): Adding t.RoleAccountMember to the UpdateCluster action increases the scope of who can perform this action. Ensure that this aligns with the intended permission model and security posture, as it allows a broader set of roles to modify cluster configurations.
| type Domain interface { | ||
| SetRemoteLoginAuthHeader(ctx context.Context, loginId repos.ID, authHeader string) error | ||
| GetRemoteLogin(ctx context.Context, loginId repos.ID, secret string) (*RemoteLogin, error) | ||
| GetRemoteLogin(ctx context.Context, loginId repos.ID, secret string) (*entities.RemoteLogin, error) | ||
| CreateRemoteLogin(ctx context.Context, secret string) (repos.ID, error) | ||
|
|
There was a problem hiding this comment.
suggestion (code_refinement): The transition to using entities.RemoteLogin instead of RemoteLogin directly is a good practice for encapsulating domain logic. However, ensure that all necessary fields and methods are correctly migrated to the entities package to avoid runtime issues.
…d-users [KLO-238] Fix/auth login for non verified users
1 participant
Resolves kloudlite/kloudlite#65