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

Dalai2547/sucu 63 file get all files typedocs #19

Open
wants to merge 6 commits into
base: dev
Choose a base branch
from

Conversation

D33102
Copy link
Contributor

@D33102 D33102 commented Nov 1, 2024

Done??

@D33102 D33102 self-assigned this Nov 1, 2024
@wiraphatys
Copy link
Contributor

@D33102 ยังไม่เสร็จนิพี่

Copy link
Contributor

@wiraphatys wiraphatys left a comment

Choose a reason for hiding this comment

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

lgtm

@bookpanda
Copy link
Contributor

attachmentRouter.Get("/role/:role_id", httpHandler.Attachment().GetAllAttachmentsByRole)
The current implementation would mean anyone can GET /role/superadmin na. If that's correct, then it looks good to me.

If not:
This is for getting attachments by role from user's session right?
To get role of user's session, we get it from access tokens passed into Auth header or http-only cookies

@bookpanda
Copy link
Contributor

@wiraphatys

@wiraphatys
Copy link
Contributor

@bookpanda sorry bro, you are right kub. the correct way should be add middleware isLogin() to this router

@wiraphatys wiraphatys self-requested a review November 16, 2024 17:10
@bookpanda
Copy link
Contributor

Thanks, so shall we wait for the auth functionality to be done first right? You can move the get all attachments to another branch if you wanna get that merged first na @D33102

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants