-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
studio: Add Studio authentication to DVC #10074
Merged
Merged
Changes from 22 commits
Commits
Show all changes
27 commits
Select commit
Hold shift + click to select a range
77d802a
Add Studio authentication to DVC
amritghimire 450e7f3
Add authentication validation and error handling to DVC Studio
amritghimire 29ed14b
Add logout functionality to DVC Studio commands
amritghimire 941764a
Improve User Authentication process for DVC Studio
amritghimire bca1510
Add unit tests for auth commands
amritghimire 0db0181
Update unit tests for auth commands
amritghimire 7e214ed
Add unit tests and modify authentication function in studio utils
amritghimire 7a956c4
Refactor mock_response usage in tests and add test_auth.py
amritghimire c57a673
Refactor error handling in `dvc/utils/studio.py`
amritghimire b94e858
Replace `auth` module with `studio` in DVC
amritghimire c5b08e4
Remove device login methods in utils/studio
amritghimire fb03545
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] 7d5315d
Update dvc-studio-client dependency in pyproject.toml
amritghimire 0be99a8
Update DVC Studio components and simplify authentication process
amritghimire a2fe144
Remove unnecessary tests and update StudioClient methods
amritghimire 1781e7d
Update token success message in dvc/commands/studio.py
amritghimire 5fd0877
Merge branch 'main' into device-auth
amritghimire 8776227
Refactor tests in `test_studio.py` for better readability
amritghimire 0b1fba8
Merge branch 'main' into device-auth
amritghimire 3eb26d1
Updated terminology in `studio.py` and `test_studio.py`
amritghimire c269930
Change 'dvc' to 'DVC' in `studio.py`
amritghimire 8423155
Improve Studio commands information clarity
amritghimire 98dc405
Not relevant anymore
amritghimire 935b7e0
Change argument name in studio login
amritghimire e893145
Modify flag for Studio login function
amritghimire eb6194b
Merge branch 'main' into device-auth
amritghimire 46c5250
Apply suggestions from code review
skshetry File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,176 @@ | ||
import argparse | ||
import os | ||
|
||
from funcy import get_in | ||
|
||
from dvc.cli.utils import append_doc_link, fix_subparsers | ||
from dvc.commands.config import CmdConfig | ||
from dvc.log import logger | ||
|
||
logger = logger.getChild(__name__) | ||
|
||
|
||
class CmdStudioLogin(CmdConfig): | ||
def run(self): | ||
from dvc_studio_client.auth import StudioAuthError, get_access_token | ||
|
||
from dvc.env import DVC_STUDIO_URL | ||
from dvc.ui import ui | ||
from dvc.utils.studio import STUDIO_URL | ||
|
||
name = self.args.name | ||
hostname = self.args.hostname or os.environ.get(DVC_STUDIO_URL) or STUDIO_URL | ||
scopes = self.args.scopes | ||
|
||
try: | ||
token_name, access_token = get_access_token( | ||
token_name=name, | ||
hostname=hostname, | ||
scopes=scopes, | ||
use_device_code=self.args.use_device_code, | ||
client_name="DVC", | ||
) | ||
except StudioAuthError as e: | ||
ui.error_write(str(e)) | ||
return 1 | ||
|
||
self.save_config(hostname, access_token) | ||
ui.write( | ||
"Authentication has been successfully completed." | ||
"The generated token will now be accessible as" | ||
f" {token_name} in the user's Studio profile." | ||
) | ||
return 0 | ||
|
||
def save_config(self, hostname, token): | ||
with self.config.edit("global") as conf: | ||
conf["studio"]["token"] = token | ||
conf["studio"]["url"] = hostname | ||
|
||
|
||
class CmdStudioLogout(CmdConfig): | ||
def run(self): | ||
from dvc.ui import ui | ||
|
||
with self.config.edit("global") as conf: | ||
if not get_in(conf, ["studio", "token"]): | ||
ui.error_write("Not logged in to Studio.") | ||
return 1 | ||
|
||
del conf["studio"]["token"] | ||
|
||
ui.write("Logged out from Studio") | ||
return 0 | ||
|
||
|
||
class CmdStudioToken(CmdConfig): | ||
def run(self): | ||
from dvc.ui import ui | ||
|
||
conf = self.config.read("global") | ||
token = get_in(conf, ["studio", "token"]) | ||
if not token: | ||
ui.error_write("Not logged in to Studio.") | ||
return 1 | ||
|
||
ui.write(token) | ||
return 0 | ||
|
||
|
||
def add_parser(subparsers, parent_parser): | ||
STUDIO_HELP = "Commands to authenticate DVC with Iterative Studio" | ||
STUDIO_DESCRIPTION = ( | ||
"Authenticate DVC with Studio and set the token." | ||
" Once this token has been properly configured,\n" | ||
" DVC will utilize it for seamlessly sharing live experiments\n" | ||
" and sending notifications to Studio regarding any experiments" | ||
" that have been pushed." | ||
) | ||
|
||
studio_parser = subparsers.add_parser( | ||
"studio", | ||
parents=[parent_parser], | ||
description=append_doc_link(STUDIO_DESCRIPTION, "studio"), | ||
help=STUDIO_HELP, | ||
formatter_class=argparse.RawDescriptionHelpFormatter, | ||
) | ||
studio_subparser = studio_parser.add_subparsers( | ||
dest="cmd", | ||
help="Use `DVC studio CMD --help` to display command-specific help.", | ||
) | ||
fix_subparsers(studio_subparser) | ||
|
||
STUDIO_LOGIN_HELP = "Authenticate DVC with Studio host" | ||
STUDIO_LOGIN_DESCRIPTION = ( | ||
"By default, this command authenticates the DVC with Studio\n" | ||
" using default scopes and assigns a random name as the token name." | ||
) | ||
login_parser = studio_subparser.add_parser( | ||
"login", | ||
parents=[parent_parser], | ||
description=append_doc_link(STUDIO_LOGIN_DESCRIPTION, "studio/login"), | ||
help=STUDIO_LOGIN_HELP, | ||
formatter_class=argparse.RawDescriptionHelpFormatter, | ||
) | ||
|
||
login_parser.add_argument( | ||
"-H", | ||
"--hostname", | ||
action="store", | ||
default=None, | ||
help="The hostname of the Studio instance to authenticate with.", | ||
) | ||
login_parser.add_argument( | ||
"-s", | ||
"--scopes", | ||
action="store", | ||
default=None, | ||
help="The scopes for the authentication token. ", | ||
) | ||
|
||
login_parser.add_argument( | ||
"-n", | ||
"--name", | ||
action="store", | ||
default=None, | ||
help="The name of the authentication token. It will be used to\n" | ||
"identify token shown in Studio profile.", | ||
) | ||
|
||
login_parser.add_argument( | ||
"-d", | ||
"--use-device-code", | ||
amritghimire marked this conversation as resolved.
Show resolved
Hide resolved
|
||
action="store_true", | ||
default=False, | ||
help="Use authentication flow based on user code.\n" | ||
"You will be presented with user code to enter in browser.\n" | ||
"DVC will also use this if it cannot launch browser on your behalf.", | ||
) | ||
login_parser.set_defaults(func=CmdStudioLogin) | ||
|
||
STUDIO_LOGOUT_HELP = "Logout user from Studio" | ||
STUDIO_LOGOUT_DESCRIPTION = ( | ||
"This removes the studio token from your global config.\n" | ||
) | ||
|
||
logout_parser = studio_subparser.add_parser( | ||
"logout", | ||
parents=[parent_parser], | ||
description=append_doc_link(STUDIO_LOGOUT_DESCRIPTION, "studio/logout"), | ||
help=STUDIO_LOGOUT_HELP, | ||
formatter_class=argparse.RawDescriptionHelpFormatter, | ||
) | ||
|
||
logout_parser.set_defaults(func=CmdStudioLogout) | ||
|
||
STUDIO_TOKEN_HELP = "View the token dvc uses to contact Studio" # noqa: S105 # nosec B105 | ||
|
||
logout_parser = studio_subparser.add_parser( | ||
"token", | ||
parents=[parent_parser], | ||
description=append_doc_link(STUDIO_TOKEN_HELP, "studio/token"), | ||
help=STUDIO_TOKEN_HELP, | ||
formatter_class=argparse.RawDescriptionHelpFormatter, | ||
) | ||
|
||
logout_parser.set_defaults(func=CmdStudioToken) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,81 @@ | ||
from dvc_studio_client.auth import AuthenticationExpired | ||
|
||
from dvc.cli import main | ||
from dvc.utils.studio import STUDIO_URL | ||
|
||
|
||
def test_studio_login_token_check_failed(mocker): | ||
mocker.patch( | ||
"dvc_studio_client.auth.get_access_token", side_effect=AuthenticationExpired | ||
) | ||
|
||
assert main(["studio", "login"]) == 1 | ||
|
||
|
||
def test_studio_login_success(mocker, dvc): | ||
mocker.patch( | ||
"dvc_studio_client.auth.get_access_token", | ||
return_value=("token_name", "isat_access_token"), | ||
) | ||
|
||
assert main(["studio", "login"]) == 0 | ||
|
||
config = dvc.config.load_one("global") | ||
assert config["studio"]["token"] == "isat_access_token" | ||
assert config["studio"]["url"] == STUDIO_URL | ||
|
||
|
||
def test_studio_login_arguments(mocker): | ||
mock = mocker.patch( | ||
"dvc_studio_client.auth.get_access_token", | ||
return_value=("token_name", "isat_access_token"), | ||
) | ||
|
||
assert ( | ||
main( | ||
[ | ||
"studio", | ||
"login", | ||
"--name", | ||
"token_name", | ||
"--hostname", | ||
"https://example.com", | ||
"--scopes", | ||
"experiments", | ||
"--use-device-code", | ||
] | ||
) | ||
== 0 | ||
) | ||
|
||
mock.assert_called_with( | ||
token_name="token_name", | ||
hostname="https://example.com", | ||
scopes="experiments", | ||
use_device_code=True, | ||
client_name="DVC", | ||
) | ||
|
||
|
||
def test_studio_logout(dvc): | ||
with dvc.config.edit("global") as conf: | ||
conf["studio"]["token"] = "isat_access_token" | ||
|
||
assert main(["studio", "logout"]) == 0 | ||
config = dvc.config.load_one("global") | ||
assert "token" not in config["studio"] | ||
|
||
assert main(["studio", "logout"]) == 1 | ||
|
||
|
||
def test_studio_token(dvc, capsys): | ||
with dvc.config.edit("global") as conf: | ||
conf["studio"]["token"] = "isat_access_token" | ||
|
||
assert main(["studio", "token"]) == 0 | ||
assert capsys.readouterr().out == "isat_access_token\n" | ||
|
||
with dvc.config.edit("global") as conf: | ||
del conf["studio"]["token"] | ||
|
||
assert main(["studio", "token"]) == 1 |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we wait for a user input before opening a browser? @shcheklein
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The one that I remember is
az login
, and I think it was not waiting for any input. We can check a few other tools. I don't have a strong opinion on this (means probably I would err to keep it simpler and faster unless we have some security concern, etc)