Skip to content

Commit

Permalink
Rework the User table (#3251)
Browse files Browse the repository at this point in the history
* Rework the User table

Rework involves the following things:

1. Modifications to the User table:
    - Drop the password column
    - Drop the auth_tokens relationship (will be later replaced with API keys table)
    - Add a column to keep track of OIDC_IDs of the user
    - Add the user relationship to the Datasets and Metadata table

2. Removed the user API

3. Changes to the user_management_cli file:
    - Kept only list users functionality.

PBENCH-1080
  • Loading branch information
npalaska committed Mar 31, 2023
1 parent 06b0718 commit 16ffcb8
Show file tree
Hide file tree
Showing 27 changed files with 602 additions and 1,305 deletions.
179 changes: 4 additions & 175 deletions lib/pbench/cli/server/user_management.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,12 @@
from pbench.cli import pass_cli_context
from pbench.cli.server import config_setup
from pbench.cli.server.options import common_options
from pbench.server.database.models.users import Roles, User
from pbench.server.database.models.users import User

USER_LIST_ROW_FORMAT = "{0:15}\t{1:15}\t{2:15}\t{3:15}\t{4:20}"
USER_LIST_HEADER_ROW = USER_LIST_ROW_FORMAT.format(
"Username", "First Name", "Last Name", "Registered On", "Email"
)
USER_LIST_ROW_FORMAT = "{0:15}\t{1:36}"
USER_LIST_HEADER_ROW = USER_LIST_ROW_FORMAT.format("Username", "OIDC ID")


# User create CLI
@click.group("user_group")
@click.version_option()
@pass_cli_context
Expand All @@ -22,94 +19,6 @@ def user_command_cli(context):
pass


@user_command_cli.command()
@pass_cli_context
@click.option(
"--username",
prompt=True,
required=True,
help="pbench server account username (will prompt if unspecified)",
)
@click.option(
"--password",
prompt=True,
hide_input=True,
required=True,
help="pbench server account password (will prompt if unspecified)",
)
@click.option(
"--email",
prompt=True,
required=True,
help="pbench server account email (will prompt if unspecified)",
)
@click.option(
"--first-name",
required=False,
help="pbench server account first name (will prompt if unspecified)",
)
@click.option(
"--last-name",
required=False,
help="pbench server account last name (will prompt if unspecified)",
)
@click.option(
"--role",
type=click.Choice([role.name for role in Roles], case_sensitive=False),
required=False,
help="Optional role of the user such as Admin",
)
@common_options
def user_create(
context: object,
username: str,
password: str,
email: str,
first_name: str,
last_name: str,
role: str,
) -> None:
try:
config_setup(context)
user = User(
username=username,
password=password,
first_name=first_name,
last_name=last_name,
email=email,
role=role if role else "",
)
user.add()
if user.is_admin():
click.echo(f"Admin user {username} registered")
else:
click.echo(f"User {username} registered")
rv = 0
except Exception as exc:
click.echo(exc, err=True)
rv = 2 if isinstance(exc, BadConfig) else 1

click.get_current_context().exit(rv)


# User delete CLI
@user_command_cli.command()
@common_options
@click.argument("username")
@pass_cli_context
def user_delete(context: object, username: str) -> None:
try:
# Delete the the user with specified username
config_setup(context)
User.delete(username=username)
rv = 0
except Exception as exc:
click.echo(exc, err=True)
rv = 2 if isinstance(exc, BadConfig) else 1

click.get_current_context().exit(rv)


# Users list CLI
@user_command_cli.command()
@common_options
Expand All @@ -126,10 +35,7 @@ def user_list(context: object) -> None:
click.echo(
USER_LIST_ROW_FORMAT.format(
user.username,
user.first_name,
user.last_name,
user.registered_on.strftime("%Y-%m-%d"),
user.email,
user.id,
)
)

Expand All @@ -139,80 +45,3 @@ def user_list(context: object) -> None:
rv = 2 if isinstance(exc, BadConfig) else 1

click.get_current_context().exit(rv)


# User update CLI
@user_command_cli.command()
@common_options
@click.argument("updateuser")
@click.option(
"--username",
required=False,
help="Specify the new username",
)
@click.option(
"--email",
required=False,
help="Specify the new email",
)
@click.option(
"--first-name",
required=False,
help="Specify the new first name",
)
@click.option(
"--last-name",
required=False,
help="Specify the new last name",
)
@click.option(
"--role",
required=False,
type=click.Choice([role.name for role in Roles], case_sensitive=False),
help="Specify the new role",
)
@pass_cli_context
def user_update(
context: object,
updateuser: str,
username: str,
first_name: str,
last_name: str,
email: str,
role: str,
) -> None:
try:
config_setup(context)
# Query the user
user = User.query(username=updateuser)

if user is None:
click.echo(f"User {updateuser} doesn't exist")
rv = 1
else:
dict_to_update = {}
if username:
dict_to_update["username"] = username

if first_name:
dict_to_update["first_name"] = first_name

if last_name:
dict_to_update["last_name"] = last_name

if email:
dict_to_update["email"] = email

if role:
dict_to_update["role"] = role

# Update the user
user.update(**dict_to_update)

click.echo(f"User {updateuser} updated")
rv = 0
except Exception as exc:
click.echo(exc, err=True)
rv = 2 if isinstance(exc, BadConfig) else 1

click.get_current_context().exit(rv)
18 changes: 12 additions & 6 deletions lib/pbench/client/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,13 +49,9 @@ class API(Enum):
DATASETS_SEARCH = "datasets_search"
DATASETS_VALUES = "datasets_values"
ENDPOINTS = "endpoints"
LOGIN = "login"
LOGOUT = "logout"
REGISTER = "register"
SERVER_AUDIT = "server_audit"
SERVER_SETTINGS = "server_settings"
UPLOAD = "upload"
USER = "user"


class PbenchServerClient:
Expand Down Expand Up @@ -205,7 +201,7 @@ def put(
api: API,
uri_params: Optional[JSONOBJECT] = None,
*,
json: Optional[dict[str, str]] = None,
json: Optional[JSONOBJECT] = None,
headers: Optional[dict[str, str]] = None,
raise_error: bool = True,
**kwargs,
Expand Down Expand Up @@ -240,7 +236,7 @@ def post(
api: API,
uri_params: Optional[JSONOBJECT] = None,
*,
json: Optional[dict[str, str]] = None,
json: Optional[JSONOBJECT] = None,
headers: Optional[dict[str, str]] = None,
raise_error: bool = True,
**kwargs,
Expand Down Expand Up @@ -443,6 +439,16 @@ def get_list(self, **kwargs) -> Iterator[Dataset]:
break
json = self.get(uri=next_url).json()

def get_user(self, username: str, add_auth_header: bool = True) -> JSONOBJECT:
""" """
if add_auth_header:
return self.get(
api=API.USER, uri_params={"target_username": username}
).json()
response = self.session.get(self._uri(API.USER, {"target_username": username}))
response.raise_for_status()
return response.json()

def get_metadata(self, dataset_id: str, metadata: list[str]) -> JSONOBJECT:
"""Return requested metadata for a specified dataset.
Expand Down
10 changes: 4 additions & 6 deletions lib/pbench/client/oidc_admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,17 +102,16 @@ def user_login(self, client_id: str, username: str, password: str) -> dict:
data = {
"client_id": client_id,
"grant_type": "password",
"scope": "profile email",
"scope": "openid profile email",
"username": username,
"password": password,
}
return self.post(path=url_path, data=data).json()

def get_user(self, username: str, token: str) -> dict:
def get_user(self, token: str) -> dict:
"""Get the OIDC user representation dict.
Args:
username: username to query
token: access_token string to validate
Returns:
Expand All @@ -132,10 +131,9 @@ def get_user(self, username: str, token: str) -> dict:
}
"""
response = self.get(
f"admin/realms/{self.OIDC_REALM}/users",
f"/realms/{self.OIDC_REALM}/protocol/openid-connect/userinfo",
headers={"Authorization": f"Bearer {token}"},
username=username,
)
if response.status_code == HTTPStatus.OK:
return response.json()[0]
return response.json()
return {}
24 changes: 0 additions & 24 deletions lib/pbench/server/api/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
from pbench.server.api.resources.server_audit import ServerAudit
from pbench.server.api.resources.server_settings import ServerSettings
from pbench.server.api.resources.upload_api import Upload
from pbench.server.api.resources.users_api import Login, Logout, RegisterUser, UserAPI
import pbench.server.auth.auth as Auth
from pbench.server.database import init_db
from pbench.server.database.database import Database
Expand Down Expand Up @@ -131,24 +130,6 @@ def register_endpoints(api: Api, app: Flask, config: PbenchServerConfig):
endpoint="endpoints",
resource_class_args=(config,),
)
api.add_resource(
Login,
f"{base_uri}/login",
endpoint="login",
resource_class_args=(config,),
)
api.add_resource(
Logout,
f"{base_uri}/logout",
endpoint="logout",
resource_class_args=(config,),
)
api.add_resource(
RegisterUser,
f"{base_uri}/register",
endpoint="register",
resource_class_args=(config,),
)
api.add_resource(
ServerAudit,
f"{base_uri}/server/audit",
Expand All @@ -163,11 +144,6 @@ def register_endpoints(api: Api, app: Flask, config: PbenchServerConfig):
endpoint="server_settings",
resource_class_args=(config,),
)
api.add_resource(
UserAPI,
f"{base_uri}/user/<string:target_username>",
endpoint="user",
)
api.add_resource(
Upload,
f"{base_uri}/upload/<string:filename>",
Expand Down
14 changes: 7 additions & 7 deletions lib/pbench/server/api/resources/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,7 @@ def convert_username(value: Union[str, None], _) -> Union[str, None]:
if not user:
raise ConversionError(value, "username", http_status=HTTPStatus.NOT_FOUND)

return str(user.id)
return user.id


def convert_dataset(value: str, _) -> Dataset:
Expand Down Expand Up @@ -1579,11 +1579,11 @@ def _get_dataset_metadata(
metadata = {}
for i in requested_items:
native_key = Metadata.get_native_key(i)
user_id = None
user: Optional[User] = None
if native_key == Metadata.USER:
user_id = Auth.get_current_user_id()
user = Auth.token_auth.current_user()
try:
metadata[i] = Metadata.getvalue(dataset=dataset, key=i, user_id=user_id)
metadata[i] = Metadata.getvalue(dataset=dataset, key=i, user=user)
except MetadataError:
metadata[i] = None

Expand All @@ -1610,11 +1610,11 @@ def _set_dataset_metadata(
fail: dict[str, str] = {}
for k, v in metadata.items():
native_key = Metadata.get_native_key(k)
user_id = None
user: Optional[User] = None
if native_key == Metadata.USER:
user_id = Auth.get_current_user_id()
user = Auth.token_auth.current_user()
try:
Metadata.setvalue(key=k, value=v, dataset=dataset, user_id=user_id)
Metadata.setvalue(key=k, value=v, dataset=dataset, user=user)
except MetadataError as e:
fail[k] = str(e)
return fail
Expand Down
1 change: 1 addition & 0 deletions lib/pbench/server/api/resources/datasets_metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
Metadata,
MetadataBadValue,
MetadataError,
User,
)


Expand Down
Loading

0 comments on commit 16ffcb8

Please sign in to comment.