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

Define REST API Spec for Consistent Usage of Project Key #879

Open
devleejb opened this issue Jun 2, 2024 · 8 comments
Open

Define REST API Spec for Consistent Usage of Project Key #879

devleejb opened this issue Jun 2, 2024 · 8 comments
Assignees
Labels
enhancement 🌟 New feature or request good first issue 🐤 Good for newcomers

Comments

@devleejb
Copy link
Member

devleejb commented Jun 2, 2024

What would you like to be added:
Currently, when sending a request to /yorkie.v1.AdminService/GetDocument, even if the project key is included in the header, the project's name still needs to be included in the request body. This inconsistency makes it challenging for users to interact with the API. To address this issue, it is necessary to define a clear REST API specification that ensures consistent usage of the project key in requests.

Why is this needed:
Having a defined REST API spec will make it easier for users to interact with the API and ensure a more intuitive usage of the project key in requests.

@hackerwins
Copy link
Member

Related to #813

@Wu22e
Copy link
Contributor

Wu22e commented Jul 18, 2024

I'm interested in this issue. Can I work on it?

@krapie
Copy link
Member

krapie commented Jul 19, 2024

@Wu22e As always, feel free to discuss with us while designing the specification.

@devleejb devleejb moved this from Backlog to In progress in Yorkie Project - 2024 Jul 20, 2024
@krapie
Copy link
Member

krapie commented Jul 25, 2024

@Wu22e @devleejb how about we discuss about this issue together to decide the spec for implementation?

@devleejb
Copy link
Member Author

devleejb commented Jul 27, 2024

It's a good idea!

It seems that we should list up all APIs that can be called with project key.
For example, /yorkie.v1.AdminService/GetDocument, /yorkie.v1.AdminService/GetDocuments, ...

Maybe, we have to discuss about it with this issue(#813).
How about discussing it in discord channel?

@Wu22e
Copy link
Contributor

Wu22e commented Aug 2, 2024

Secret key validation can be performed for all AdminService APIs except LogIn, SignUp, ChangePassword, and DeleteAccount.

Current Situation

In the authenticate function of yorkie/server/rpc/interceptors/admin.go, the authorization value in the header is used to distinguish between a token and a secret key (priority: token first, then secret key).

If a user token is used in the authorization header for GetProjects, project-level identification is not possible.
If a secret key is used, project-level identification is possible, but it allows access to all projects under that user’s identity.

Proposed Solution:

  • Only include the User Token in the Authorization header.
  • The project list retrieval API will use the owner ID to fetch all projects with the user token.
  1. For single project retrieval, use the x-secret-key in the header to verify if the project corresponding to the secret key matches the requested project name. Specific APIs will pass through an interceptor, e.g. AdminSecretKeyInterceptor.

  2. Or, store the project found using x-secret-key in the context and ensure that the project in the request matches the project corresponding to the secret key when fetching projectInfo.

Other methods may also exist, so it would be great to discuss further!

@krapie
Copy link
Member

krapie commented Sep 5, 2024

@Wu22e @devleejb @hackerwins Seems like we have lost our conversation afterward. Any thoughts?

@devleejb
Copy link
Member Author

devleejb commented Sep 6, 2024

Sorry for the delayed response.

I think it's a good approach to separate the field in the authorization header.

@hackerwins @krapie
What do you think about method 2? Also, I believe we can remove the project_name field if a project secret key exists, as it would be redundant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement 🌟 New feature or request good first issue 🐤 Good for newcomers
Projects
Status: In progress
Status: Todo
Development

No branches or pull requests

4 participants