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

Implement '--expiry' flag for 'mc admin user svcacct add & edit' #4570

Merged
merged 4 commits into from
May 30, 2023

Conversation

kaankabalak
Copy link
Collaborator

@kaankabalak kaankabalak commented May 12, 2023

Description

This commit implements the --expiry flag for the mc admin user svcacct add and mc admin user svcacct edit commands, allowing users to set expiration dates for service accounts through mc.

The expiry date can be set in the following formats:

  • 2023-06-24
  • 2023-06-24T10:00
  • 2023-06-24T10:00:00
  • 2023-06-24T10:00:00Z
  • 2023-06-24T10:00:00-07:00

Motivation and Context

We would like to implement this functionality as the APIs to set/edit expiration for service accounts already have been implemented in minio/madmin-go#170.

How to test this PR?

  • Build the mc binary from this branch
  • Create users called foobar1, foobar2, foobar3, foobar4 and foobar5 using the mc admin user add command.
  • Make sure that the service accounts are created successfully for the following cases where the --expiry flag is present:
$ ./mc admin user svcacct add myminio foobar1 --expiry 2023-06-24                      
$ ./mc admin user svcacct add myminio foobar2 --expiry 2023-06-24T10:00                
$ ./mc admin user svcacct add myminio foobar3 --expiry 2023-06-24T10:00:00             
$ ./mc admin user svcacct add myminio foobar4 --expiry 2023-06-24T10:00:00Z            
$ ./mc admin user svcacct add myminio foobar5 --expiry 2023-06-24T10:00:00-07:00
  • Make sure that service accounts with expiry values that cannot be parsed, such as the following, are not created:
$ ./mc admin user svcacct add myminio foobar1 --expiry 2023
$ ./mc admin user svcacct add myminio foobar1 --expiry 2023-06
$ ./mc admin user svcacct add myminio foobar1 --expiry 2023-06-24T
$ ./mc admin user svcacct add myminio foobar1 --expiry 2023-06-24T10
$ ./mc admin user svcacct add myminio foobar1 --expiry 2023-06-24T10:00:00Z07:00
$ ./mc admin user svcacct add myminio foobar1 --expiry RANDOM_TEXT
  • Make sure that service accounts can be created without the --expiry flag
  • For a service account with Access Key J123C4ZXEQN8RK6ND35I, make sure that it can be successfully edited as follows:
$ ./mc admin user svcacct edit myminio 'J123C4ZXEQN8RK6ND35I' --expiry 2023-08-24                      
$ ./mc admin user svcacct edit myminio 'J123C4ZXEQN8RK6ND35I' --expiry 2023-09-24T10:00                
$ ./mc admin user svcacct edit myminio 'J123C4ZXEQN8RK6ND35I' --expiry 2023-10-24T10:00:00             
$ ./mc admin user svcacct edit myminio 'J123C4ZXEQN8RK6ND35I' --expiry 2023-11-24T10:00:00Z            
$ ./mc admin user svcacct edit myminio 'J123C4ZXEQN8RK6ND35I' --expiry 2023-12-24T10:00:00-07:00
  • Make sure that service accounts with expiry values that cannot be parsed, such as the following, are not edited:
$ ./mc admin user svcacct edit myminio 'J123C4ZXEQN8RK6ND35I' --expiry 2023
$ ./mc admin user svcacct edit myminio 'J123C4ZXEQN8RK6ND35I' --expiry 2023-06
$ ./mc admin user svcacct edit myminio 'J123C4ZXEQN8RK6ND35I' --expiry 2023-06-24T
$ ./mc admin user svcacct edit myminio 'J123C4ZXEQN8RK6ND35I' --expiry 2023-06-24T10
$ ./mc admin user svcacct edit myminio 'J123C4ZXEQN8RK6ND35I' --expiry 2023-06-24T10:00:00Z07:00
$ ./mc admin user svcacct edit myminio 'J123C4ZXEQN8RK6ND35I' --expiry RANDOM_TEXT
  • Make sure that service accounts can be edited without the --expiry flag

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Optimization (provides speedup with no functional changes)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • Fixes a regression (If yes, please add commit-id or PR # here)
  • Unit tests added/updated
  • Internal documentation updated
  • Create a documentation update request here

@shtripat
Copy link
Contributor

@kaankabalak while verifying edit without --expiry I see errors as below

$ ./mc admin user svcacct edit myminio foobar1 --secret-key "foobar11234"
mc: <ERROR> Unable to edit the specified service account. We encountered an internal error, please try again. (invalid argument).

$ ./mc admin user svcacct edit myminio foobar1 --expiry 2023-08-24
mc: <ERROR> Unable to edit the specified service account. We encountered an internal error, please try again. (invalid argument).

Server side I can see error as below

API: UpdateServiceAccount()
Time: 04:12:25 UTC 05/12/2023
DeploymentID: 27d30233-9448-472f-8249-e8f48dc4d953
RequestID: 175E4A108D0CDE88
RemoteHost: [::1]
Host: localhost:9000
UserAgent: MinIO (linux; amd64) madmin-go/2.0.0 mc/DEVELOPMENT.2023-05-12T00-56-01Z
Error: invalid argument (*errors.errorString)
       6: internal/logger/logger.go:258:logger.LogIf()
       5: cmd/api-errors.go:2291:cmd.toAPIErrorCode()
       4: cmd/admin-handler-utils.go:233:cmd.toAdminAPIErrCode()
       3: cmd/admin-handler-utils.go:220:cmd.toAdminAPIErr()
       2: cmd/admin-handlers-users.go:877:cmd.adminAPIHandlers.UpdateServiceAccount()
       1: net/http/server.go:2122:http.HandlerFunc.ServeHTTP()

@kaankabalak
Copy link
Collaborator Author

@kaankabalak while verifying edit without --expiry I see errors as below

Hi @shtripat, I can't seem to reproduce this for now, are you using the latest MinIO Server code while running these commands?

@shtripat
Copy link
Contributor

shtripat commented May 12, 2023

./mc admin user svcacct edit myminio foobar1 --secret-key "foobar11234"

Yes, latest master build of minio and this PR branch of mc as below

./minio -version
minio version DEVELOPMENT.2023-05-12T00-41-33Z (commit-id=f5a20a5d06b7980c85fd010a52a0f8b2ce9c8ed7)
Runtime: go1.20.2 linux/amd64
License: GNU AGPLv3 <https://www.gnu.org/licenses/agpl-3.0.html>
Copyright: 2015-2023 MinIO, Inc.
./mc --version
mc version DEVELOPMENT.2023-05-12T00-56-01Z (commit-id=ca75c63029fcccd6b942d32ed727ef21ff4a0b32)
Runtime: go1.20.2 linux/amd64
Copyright (c) 2015-2023 MinIO, Inc.
License GNU AGPLv3 <https://www.gnu.org/licenses/agpl-3.0.html>

./mc admin user svcacct edit myminio foobar1 --secret-key "foobar11234" 
mc: <ERROR> Unable to edit the specified service account. We encountered an internal error, please try again. (invalid argument).

@kaankabalak
Copy link
Collaborator Author

Thanks @shtripat, I will investigate further and get back to you 👍

@kaankabalak
Copy link
Collaborator Author

kaankabalak commented May 16, 2023

mc admin user svcacct add myminio foobar1 --expiry 2023-06-24

My apologies @shtripat, looks like I have pasted the edit command incorrectly on the PR instructions. The following should work for a service account with access key J123C4ZXEQN8RK6ND35I. I have updated all the instructions accordingly in the first message of the PR:

./mc admin user svcacct edit myminio 'J123C4ZXEQN8RK6ND35I' --expiry 2023-09-28

@shtripat
Copy link
Contributor

shtripat commented May 17, 2023

--expiry 2023-09-28

Ah yes, this works as expected. I should have looked at help of the command :(

Copy link
Contributor

@shtripat shtripat left a comment

Choose a reason for hiding this comment

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

LGTM. Verified the changes.

@shtripat
Copy link
Contributor

@kaankabalak please resolve the conflicts.

@kaankabalak
Copy link
Collaborator Author

@kaankabalak please resolve the conflicts.

Done @shtripat 👍

@kaankabalak kaankabalak requested a review from vadmeste May 26, 2023 22:04
@harshavardhana harshavardhana merged commit ca1d007 into minio:master May 30, 2023
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