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

Multi auth provider credentials #93

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
40 changes: 28 additions & 12 deletions mars-cli/mars_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -197,10 +197,22 @@ def cli(ctx, development):

@cli.command()
@click.option(
"--credential-service-name", type=click.STRING, help="service name from the keyring"
"--webin-username",
type=click.STRING,
help="Username for webin authentication",
envvar="WEBIN_USERNAME",
)
@click.option(
"--metabolights-username",
type=click.STRING,
help="Username for MetaboLights metadata submission",
envvar="METABOLIGHTS_USERNAME",
)
@click.option(
"--username-credentials", type=click.STRING, help="Username from the keyring"
"--metabolights-ftp-username",
type=click.STRING,
help="Username for MetaboLights data submission",
envvar="METABOLIGHTS_FTP_USERNAME",
)
@click.option(
"--credentials-file",
Expand Down Expand Up @@ -247,8 +259,9 @@ def cli(ctx, development):
@click.pass_context
def submit(
ctx,
credential_service_name,
username_credentials,
webin_username,
metabolights_username,
metabolights_ftp_username,
credentials_file,
isa_json_file,
submit_to_biosamples,
Expand Down Expand Up @@ -280,8 +293,9 @@ def submit(
data_file_paths = [f.name for f in data_files] if file_transfer else []

submission(
credential_service_name,
username_credentials,
webin_username,
metabolights_username,
metabolights_ftp_username,
credentials_file,
isa_json_file.name,
target_repositories,
Expand Down Expand Up @@ -361,12 +375,14 @@ def validate_isa_json(isa_json_file, investigation_is_root, validation_schema):

@cli.command()
@click.option(
"--service-name",
type=click.STRING,
"--auth-provider",
type=click.Choice(
["webin", "metabolights_metadata", "metabolights_data"], case_sensitive=False
),
is_flag=False,
flag_value="value",
default=f"mars-cli_{datetime.now().strftime('%Y-%m-%dT%H:%M:%S')}",
help='You are advised to include service name to match the credentials to. If empty, it defaults to "mars-cli_{DATESTAMP}"',
required=True,
help="",
)
@click.argument(
"username",
Expand All @@ -380,9 +396,9 @@ def validate_isa_json(isa_json_file, investigation_is_root, validation_schema):
confirmation_prompt=True,
help="The password to store. Note: You are required to confirm the password.",
)
def set_password(service_name, username, password):
def set_password(auth_provider, username, password):
"""Store a password in the keyring."""
CredentialManager(service_name).set_password_keyring(username, password)
CredentialManager(auth_provider).set_password_keyring(username, password)


if __name__ == "__main__":
Expand Down
71 changes: 65 additions & 6 deletions mars-cli/mars_lib/authentication.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,66 @@
from typing import Optional
import io
from typing import Optional, Union
import requests
import json
from enum import Enum


class AuthProvider(Enum):
"""
Holds constants, tied to the repository authentication providers.
"""

WEBIN = "webin"
METABOLIGHTS_METADATA = "metabolights_metadata"
METABOLIGHTS_DATA = "metabolights_data"

@classmethod
def available_providers(cls):
return {item.value for item in cls}

@classmethod
def is_valid_provider(cls, provider: str):
return provider in cls.available_providers()


def load_credentials(
credentials_file: Union[io.TextIOWrapper, str]
) -> dict[str, dict[str, str]]:
"""
Validate the credentials.

Args:
credentials_file (_): The credentials in file formate.

Raises:
ValueError: If the credentials are not valid.

Returns:
dict: The credentials.
"""
if isinstance(credentials_file, str):
with open(credentials_file, "r") as file:
credentials = json.load(file)
elif isinstance(credentials_file, io.TextIOWrapper):
with open(credentials_file.name, "r") as file:
credentials = json.load(file)
else:
raise TypeError("Credentials file must be of type str or io.TextIOWrapper.")

if not all(
repo in AuthProvider.available_providers() for repo in credentials.keys()
):
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think eventually should loosen this up, e.g. if I'm only submitting the ENA, EVA and BioSamples I shouldn't need Metabolights credentials... it's fine for now though.

raise ValueError("Credentials dictionary must have keys .")

if not all(
key in ["username", "password"]
for repo, creds in credentials.items()
for key in creds.keys()
):
raise ValueError(
"Credentials dictionary must contain 'username' and 'password' keys."
)
return credentials


def get_webin_auth_token(
Expand Down Expand Up @@ -45,7 +105,10 @@ def get_webin_auth_token(

def get_metabolights_auth_token(
credentials_dict: dict[str, str],
headers: dict[str, str] = {"Content-Type": "application/x-www-form-urlencoded"},
headers: dict[str, str] = {
"Content-Type": "application/x-www-form-urlencoded",
"Accept": "application/json",
},
auth_url: str = "https://www-test.ebi.ac.uk/metabolights/mars/ws3/auth/token",
) -> Optional[str]:
"""
Expand All @@ -59,10 +122,6 @@ def get_metabolights_auth_token(
Returns:
str: The obtained token.
"""
headers = {
"Content-Type": "application/x-www-form-urlencoded",
"Accept": "application/json",
}
form_data = f'grant_type=password&username={credentials_dict["username"]}&password={credentials_dict["password"]}'
try:
response = requests.post(
Expand Down
9 changes: 7 additions & 2 deletions mars-cli/mars_lib/credential.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
import getpass
import keyring.util.platform_ as keyring_platform

from mars_lib.authentication import AuthProvider

"""
Credential Manager Module
=========================
Expand Down Expand Up @@ -52,8 +54,11 @@


class CredentialManager:
def __init__(self, service_name: str) -> None:
self.service_name = service_name
def __init__(self, auth_provider: str) -> None:
if not AuthProvider.is_valid_provider(auth_provider):
raise ValueError(f"Invalid authentication provider: {auth_provider}")

self.service_name = auth_provider

def get_credential_env(self, username: str) -> str:
"""
Expand Down
4 changes: 2 additions & 2 deletions mars-cli/mars_lib/models/isa_json.py
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ class MaterialAttribute(IsaBase):


class Study(CommentedIsaBase):
id: str = Field(alias="@id", default=None)
id: Optional[str] = Field(alias="@id", default=None)
assays: List[Assay] = []
characteristicCategories: List[MaterialAttribute] = []
description: Optional[str] = None
Expand Down Expand Up @@ -284,7 +284,7 @@ def validate_filename(cls, v: str) -> Union[str, None]:


class Investigation(CommentedIsaBase):
id: str = Field(alias="@id", default=None)
id: Optional[str] = Field(alias="@id", default=None)
description: Optional[str] = None
filename: Optional[str] = None
identifier: Optional[str] = None
Expand Down
45 changes: 30 additions & 15 deletions mars-cli/mars_lib/submit.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,12 @@
import requests
import json
from typing import Any
from mars_lib.authentication import get_metabolights_auth_token, get_webin_auth_token
from mars_lib.authentication import (
get_metabolights_auth_token,
get_webin_auth_token,
load_credentials,
AuthProvider,
)
from mars_lib.biosamples_external_references import (
get_header,
biosamples_endpoints,
Expand Down Expand Up @@ -44,8 +49,9 @@ def save_step_to_file(time_stamp: float, filename: str, isa_json: IsaJson):


def submission(
credential_service_name: str,
username_credentials: str,
webin_username: str,
metabolights_username: str,
metabolights_ftp_username: str,
credentials_file: TextIOWrapper,
isa_json_file: str,
target_repositories: list[str],
Expand All @@ -59,17 +65,24 @@ def submission(
# Get password from the credential manager
# Else:
# read credentials from file
if not (credential_service_name is None or username_credentials is None):
cm = CredentialManager(credential_service_name)
if all([webin_username, metabolights_username, metabolights_ftp_username]):
user_credentials = {
"username": username_credentials,
"password": cm.get_password_keyring(username_credentials),
cred_pair[0]: {
"username": cred_pair[1],
"password": CredentialManager(cred_pair[0]).get_password_keyring(
cred_pair[1]
),
}
for cred_pair in zip(
AuthProvider.available_providers(),
[webin_username, metabolights_username, metabolights_ftp_username],
)
}
else:
if credentials_file == "":
raise ValueError("No credentials found")

user_credentials = json.load(credentials_file)
user_credentials = load_credentials(credentials_file)

isa_json = load_isa_json(isa_json_file, investigation_is_root)

Expand Down Expand Up @@ -101,7 +114,7 @@ def submission(
# Submit to Biosamples
biosamples_result = submit_to_biosamples(
isa_json=isa_json,
biosamples_credentials=user_credentials,
biosamples_credentials=user_credentials[TargetRepository.BIOSAMPLES.value],
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this be AuthProvider.WEBIN.value instead? (And same for the ENA usages below)
Not sure if we need some kind of TargetRepository -> AuthProvider mapping, or whether just defining it implicitly in this method is enough.

biosamples_url=urls["BIOSAMPLES"]["SUBMISSION"],
webin_token_url=urls["WEBIN"]["TOKEN"],
)
Expand All @@ -124,7 +137,7 @@ def submission(
file_paths=[
Path(df) for df in data_file_map[TargetRepository.ENA.value]
],
user_credentials=user_credentials,
user_credentials=user_credentials[TargetRepository.ENA.value],
submission_url=urls["ENA"]["DATA-SUBMISSION"],
file_transfer=file_transfer,
)
Expand All @@ -135,7 +148,7 @@ def submission(
# Step 2 : submit isa-json to ena
ena_result = submit_to_ena(
isa_json=isa_json,
user_credentials=user_credentials,
user_credentials=user_credentials[TargetRepository.ENA.value],
submission_url=urls["ENA"]["SUBMISSION"],
)
print_and_log(
Expand All @@ -159,7 +172,9 @@ def submission(
file_paths=data_file_map[TargetRepository.METABOLIGHTS.value],
file_transfer=file_transfer,
isa_json=isa_json,
metabolights_credentials=user_credentials,
metabolights_credentials=user_credentials[
TargetRepository.METABOLIGHTS.value
],
metabolights_url=urls["METABOLIGHTS"]["SUBMISSION"],
metabolights_token_url=urls["METABOLIGHTS"]["TOKEN"],
)
Expand Down Expand Up @@ -252,9 +267,9 @@ def upload_to_metabolights(
"accept": "application/json",
"Authorization": f"Bearer {token}",
}
isa_json_str = isa_json.investigation.model_dump_json(
by_alias=True, exclude_none=True
)
isa_json_str = reduce_isa_json_for_target_repo(
isa_json, TargetRepository.METABOLIGHTS
).investigation.model_dump_json(by_alias=True, exclude_none=True)
json_file = io.StringIO(isa_json_str)

files = {"isa_json_file": ("isa_json.json", json_file)}
Expand Down
49 changes: 39 additions & 10 deletions mars-cli/tests/test_authentication.py
Original file line number Diff line number Diff line change
@@ -1,24 +1,53 @@
import pytest
import json
import os
from mars_lib.authentication import get_webin_auth_token
from mars_lib.authentication import (
get_webin_auth_token,
load_credentials,
get_metabolights_auth_token,
)
from requests.exceptions import HTTPError


def test_get_webin_auth_token():
fake_credentials_dict = {
fake_credentials_dict = {
"webin": {
"username": "my_fake_account",
"password": "my_super_secret_password",
},
"metabolights_metadata": {
"username": "my_fake_account",
"password": "my_super_secret_password",
},
"metabolights_data": {
"username": "my_fake_account",
"password": "my_super_secret_password",
}
},
}


def test_get_webin_auth_token():
with pytest.raises(
ValueError,
match="ERROR when generating token. See response's content below:\nBad credentials",
):
get_webin_auth_token(fake_credentials_dict)
get_webin_auth_token(fake_credentials_dict["webin"])

file = "./tests/test_credentials.json"
if os.path.exists(file) and os.path.isfile(file):
with open(file, "r") as f:
test_credentials = json.load(f)
test_credentials = load_credentials(file)

token = get_webin_auth_token(test_credentials)
token = get_webin_auth_token(test_credentials["webin"])
assert token


@pytest.mark.skipif(
not os.path.exists("./tests/test_credentials.json"),
reason="Credentials file not found",
)
def test_get_metabolights_auth_token():
credentials = load_credentials("./tests/test_credentials.json")
token = get_metabolights_auth_token(credentials["metabolights_metadata"])
assert token

# TODO: Currently an 'Internal Server Error' is returned when using the wrong credentials.
# This should be updated to return a more informative error message.
with pytest.raises(HTTPError):
get_metabolights_auth_token(fake_credentials_dict["metabolights_metadata"])
Loading