feat: create a token for another user via cli #13424
Conversation
Signed-off-by: Philip-21 <philipuzomaobiora@gmail.com>
|
All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
There was a problem hiding this comment.
@Philip-21, you must run make fmt to pass the fmt ci check. Also, please add tests to cover the newly added functionality.
codersdk/apikey.go
Outdated
| // CreateTokenForUser allows an admin to create a token on behalf of another user. | ||
| func (c *Client) CreateTokenForUser(ctx context.Context, adminID, targetUserID string, req CreateTokenRequest) (GenerateAPIKeyResponse, error) { | ||
| isAdmin, err := c.isAdmin(ctx, adminID) | ||
| if err != nil { | ||
| return GenerateAPIKeyResponse{}, fmt.Errorf("admin check failed: %w", err) | ||
| } | ||
| if !isAdmin { | ||
| return GenerateAPIKeyResponse{}, fmt.Errorf("user %s is not an admin", adminID) | ||
| } | ||
| return c.CreateToken(ctx, targetUserID, req) | ||
| } | ||
|
|
||
| // isAdmin is a placeholder function to check if a user is an admin. | ||
| func (c *Client) isAdmin(ctx context.Context, userID string) (bool, error) { | ||
| res, err := c.Request(ctx, http.MethodGet, fmt.Sprintf("/api/v2/users/%s/keys", userID), nil) | ||
| if err != nil { | ||
| return false, fmt.Errorf("unable to get user : %s", userID) | ||
| } | ||
| defer res.Body.Close() | ||
| if res.StatusCode > http.StatusCreated { | ||
| return false, ReadBodyAsError(res) | ||
| } | ||
| //extract roles, when the API returns roles from the response, ensuring system admin privileges | ||
| var roles []string | ||
| if err := json.NewDecoder(res.Body).Decode(&roles); err != nil { | ||
| return false, fmt.Errorf("failed to decode roles: %w", err) | ||
| } | ||
| for _, role := range roles { | ||
| if role == "admin" { | ||
| return true, nil | ||
| } | ||
| } | ||
| return false, nil | ||
| } | ||
|
|
There was a problem hiding this comment.
The API should return an error when the user does not have permissions to create keys for other users, so we shouldn't need to add any new codersdk functions.
cli/tokens.go
Outdated
| // Otherwise, create a token for the current user | ||
| res, err := client.CreateToken(inv.Context(), codersdk.Me, codersdk.CreateTokenRequest{ | ||
| Lifetime: tokenLifetime, |
There was a problem hiding this comment.
I think you can actually keep this the same as it was before, but add a userID flag.
e.g.
userID := codersdk.Me
if user != "" {
userID = user
}
// Use the `userID` instead of `codersdk.Me` in `client.CreateToken`.|
Thank you i will make changes |
Signed-off-by: Philip-21 <philipuzomaobiora@gmail.com>
|
Hey @kylecarbs from the changes i have made, i added another condition where a user doesnt have the permission to create a key for another user. |
Alright |
|
Hey @matifali i ran
|
|
Now i have made changes in the |
@Philip-21 Please ensure you have a working dev environment by following these instructions. I would advise using devcontainer or |
Signed-off-by: Philip-21 <philipuzomaobiora@gmail.com>
Signed-off-by: Philip-21 <philipuzomaobiora@gmail.com>
|
@matifali i ran after following the steps and using it didnt finish executing |
|
@bpmct, could you get some eyes on this PR? I am away for the next few weeks. |
|
@kylecarbs can you take another look? |
| // isAdmin is a placeholder function to check if a user is an admin. | ||
| func (c *Client) isAdmin(ctx context.Context, userID string) (bool, error) { | ||
| res, err := c.Request(ctx, http.MethodGet, fmt.Sprintf("/api/v2/users/%s/keys", userID), nil) | ||
| if err != nil { | ||
| return false, fmt.Errorf("unable to get user : %s", userID) | ||
| } | ||
| defer res.Body.Close() | ||
| if res.StatusCode > http.StatusCreated { | ||
| return false, ReadBodyAsError(res) | ||
| } | ||
| //extract roles, when the API returns roles from the response, ensuring system admin privileges | ||
| var roles []string | ||
| if err := json.NewDecoder(res.Body).Decode(&roles); err != nil { | ||
| return false, fmt.Errorf("failed to decode roles: %w", err) | ||
| } | ||
| for _, role := range roles { | ||
| if role == "admin" { | ||
| return true, nil | ||
| } | ||
| } | ||
| return false, nil | ||
| } |
There was a problem hiding this comment.
We shouldn't perform intermediary checks in our API package.
Ideally, our API returns a friendly error indicating that the user isn't an admin, which reduces the number of requests required as well.
Pr fixes for #13160