Skip to content

Commit

Permalink
Merge branch 'main' into 1646-escape-or-reject-special-characters-in-…
Browse files Browse the repository at this point in the history
…user-provided-strings
  • Loading branch information
ychiucco committed Sep 17, 2024
2 parents 5ac3837 + 18e87c8 commit 8cb2201
Show file tree
Hide file tree
Showing 16 changed files with 317 additions and 101 deletions.
30 changes: 16 additions & 14 deletions .github/workflows/oauth.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,20 @@ jobs:
ports:
- 5556:5556

env:
FRACTAL_TASKS_DIR: /dev/fractal/task
FRACTAL_RUNNER_WORKING_BASE_DIR: /dev/fractal/base_dir
FRACTAL_RUNNER_BACKEND: local
JWT_SECRET_KEY: jwt_secret_key
JWT_EXPIRE_SECONDS: 1000
DB_ENGINE: sqlite
SQLITE_PATH: /tmp/fractal.sqlite
OAUTH_DEXIDP_CLIENT_ID: client_test_id
OAUTH_DEXIDP_CLIENT_SECRET: client_test_secret
OAUTH_DEXIDP_REDIRECT_URL: http://localhost:8001/auth/dexidp/callback/
OAUTH_DEXIDP_OIDC_CONFIGURATION_ENDPOINT: http://127.0.0.1:5556/dex/.well-known/openid-configuration


steps:

- uses: actions/checkout@v4
Expand All @@ -33,21 +47,9 @@ jobs:

- name: Run Fractal
run: |
export FRACTAL_TASKS_DIR=/dev/fractal/task
export FRACTAL_RUNNER_WORKING_BASE_DIR=/dev/fractal/base_dir
export FRACTAL_RUNNER_BACKEND=local
export JWT_SECRET_KEY=jwt_secret_key
export JWT_EXPIRE_SECONDS=1000
export DB_ENGINE=sqlite
export SQLITE_PATH=fractal.sqlite
export OAUTH_DEXIDP_CLIENT_ID=client_test_id
export OAUTH_DEXIDP_CLIENT_SECRET=client_test_secret
export OAUTH_DEXIDP_REDIRECT_URL=http://localhost:8001/auth/dexidp/callback/
export OAUTH_DEXIDP_OIDC_CONFIGURATION_ENDPOINT=http://127.0.0.1:5556/dex/.well-known/openid-configuration
fractalctl set-db
fractalctl start --port 8001 &
sleep 2
- name: OAuth authentication
run: |
bash -x scripts/oauth/oauth.sh
- name: Run OAuth2 script
run: bash -x scripts/oauth/oauth.sh
8 changes: 6 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
**Note**: Numbers like (\#1234) point to closed Pull Requests on the fractal-server repository.


# 2.5.0

This release removes support for including V1 tasks in V2 workflows. This comes
Expand All @@ -15,13 +16,16 @@ runner.
* Drop v1-v2-task-compatibility admin endpoint (\#1721).
* Drop `/task-legacy/` endpoint (\#1721).
* Remove legacy task code branches from `WorkflowTaskV2` CRUD endpoints (\#1721).
* Add OAuth accounts info to `UserRead` at `.oauth_accounts` (\#1765).
* Testing:
* Improve OAuth Github Action to test OAuth account flow (\#1765).

# 2.4.2

* Runner:
* Add `--set-home` to `sudo -u` impersonation command, to fix Ubuntu18 behavior (\#1762).
* App:
* Improve logging in `fractalctl set-db` (\#1764).
* Runner:
* Add `--set-home` to `sudo -u` impersonation command, to fix Ubuntu18 behavior (\#1762).
* Testing:
* Start tests of migrations from valid v2.4.0 database (\#1764).

Expand Down
2 changes: 1 addition & 1 deletion fractal_server/__init__.py
Original file line number Diff line number Diff line change
@@ -1 +1 @@
__VERSION__ = "2.5.0a0"
__VERSION__ = "2.5.0"
16 changes: 11 additions & 5 deletions fractal_server/app/routes/auth/_aux_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,11 @@ async def _get_single_user_with_group_names(
res = await db.execute(stm_groups)
groups = res.scalars().unique().all()
group_names = [group.name for group in groups]
return UserRead(**user.model_dump(), group_names=group_names)
return UserRead(
**user.model_dump(),
group_names=group_names,
oauth_accounts=user.oauth_accounts,
)


async def _get_single_user_with_group_ids(
Expand All @@ -53,7 +57,11 @@ async def _get_single_user_with_group_ids(
res = await db.execute(stm_links)
links = res.scalars().unique().all()
group_ids = [link.group_id for link in links]
return UserRead(**user.model_dump(), group_ids=group_ids)
return UserRead(
**user.model_dump(),
group_ids=group_ids,
oauth_accounts=user.oauth_accounts,
)


async def _get_single_group_with_user_ids(
Expand Down Expand Up @@ -97,9 +105,7 @@ async def _user_or_404(user_id: int, db: AsyncSession) -> UserOAuth:
user_id: ID of the user
db: Async db session
"""
stm = select(UserOAuth).where(UserOAuth.id == user_id)
res = await db.execute(stm)
user = res.scalars().unique().one_or_none()
user = await db.get(UserOAuth, user_id, populate_existing=True)
if user is None:
raise HTTPException(
status_code=status.HTTP_404_NOT_FOUND, detail="User not found."
Expand Down
6 changes: 5 additions & 1 deletion fractal_server/app/routes/auth/current_user.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,13 @@ async def patch_current_user(
# NOTE: here it would be relevant to catch an `InvalidPasswordException`
# (from `fastapi_users.exceptions`), if we were to allow users change
# their own password

user = await user_manager.update(update, current_user, safe=True)
patched_user = schemas.model_validate(UserOAuth, user)
validated_user = schemas.model_validate(UserOAuth, user)

patched_user = await db.get(
UserOAuth, validated_user.id, populate_existing=True
)
patched_user_with_groups = await _get_single_user_with_group_names(
patched_user, db
)
Expand Down
17 changes: 9 additions & 8 deletions fractal_server/app/routes/auth/users.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,8 @@ async def get_user(
) -> UserRead:
user = await _user_or_404(user_id, db)
if group_ids:
user_with_group_ids = await _get_single_user_with_group_ids(user, db)
return user_with_group_ids
else:
return user
user = await _get_single_user_with_group_ids(user, db)
return user


@router_users.patch("/users/{user_id}/", response_model=UserRead)
Expand Down Expand Up @@ -127,9 +125,7 @@ async def patch_user(
status_code=status.HTTP_422_UNPROCESSABLE_ENTITY,
detail=error_msg,
)

patched_user = user_to_patch

elif edit_attributes:
# Modify user attributes
try:
Expand All @@ -142,7 +138,10 @@ async def patch_user(
safe=False,
request=None,
)
patched_user = schemas.model_validate(UserOAuth, user)
validated_user = schemas.model_validate(UserOAuth, user)
patched_user = await db.get(
UserOAuth, validated_user.id, populate_existing=True
)
except exceptions.InvalidPasswordException as e:
raise HTTPException(
status_code=status.HTTP_400_BAD_REQUEST,
Expand Down Expand Up @@ -189,9 +188,11 @@ async def list_users(
# https://github.com/fractal-analytics-platform/fractal-server/issues/1742
for ind, user in enumerate(user_list):
user_list[ind] = dict(
user.model_dump(),
**user.model_dump(),
oauth_accounts=user.oauth_accounts,
group_ids=[
link.group_id for link in links if link.user_id == user.id
],
)

return user_list
17 changes: 17 additions & 0 deletions fractal_server/app/schemas/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,22 @@
)


class OAuthAccountRead(BaseModel):
"""
Schema for storing essential `OAuthAccount` information within
`UserRead.oauth_accounts`.
Attributes:
id: ID of the row in fractal-owned `oauthaccount` table.
account_email: Email associated to OAuth account
oauth_name: Name of the OAuth provider (e.g. `github`)
"""

id: int
account_email: str
oauth_name: str


class UserRead(schemas.BaseUser[int]):
"""
Schema for `User` read from database.
Expand All @@ -37,6 +53,7 @@ class UserRead(schemas.BaseUser[int]):
slurm_accounts: list[str]
group_names: Optional[list[str]] = None
group_ids: Optional[list[int]] = None
oauth_accounts: list[OAuthAccountRead]


class UserUpdate(schemas.BaseUserUpdate):
Expand Down
4 changes: 2 additions & 2 deletions pyproject.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[tool.poetry]
name = "fractal-server"
version = "2.5.0a0"
version = "2.5.0"
description = "Server component of the Fractal analytics platform"
authors = [
"Tommaso Comparin <tommaso.comparin@exact-lab.it>",
Expand Down Expand Up @@ -92,7 +92,7 @@ filterwarnings = [
]

[tool.bumpver]
current_version = "2.5.0a0"
current_version = "2.5.0"
version_pattern = "MAJOR.MINOR.PATCH[PYTAGNUM]"
commit_message = "bump version {old_version} -> {new_version}"
commit = true
Expand Down
125 changes: 108 additions & 17 deletions scripts/oauth/oauth.sh
Original file line number Diff line number Diff line change
@@ -1,24 +1,115 @@
#!/bin/bash

AUTHORIZATION_URL=$(curl --silent \
http://127.0.0.1:8001/auth/dexidp/authorize/ \
| jq -r ".authorization_url"
)
# --- Functions

TOKEN=$(
curl -L --silent --output /dev/null --cookie-jar - "$AUTHORIZATION_URL" \
| grep "fastapiusersauth" | awk '{print $NF}'
)
oauth_login(){
AUTHORIZATION_URL=$(curl --silent \
http://127.0.0.1:8001/auth/dexidp/authorize/ \
| jq -r ".authorization_url"
)
TOKEN_OAUTH=$(
curl -L --silent --output /dev/null --cookie-jar - "$AUTHORIZATION_URL" \
| grep "fastapiusersauth" | awk '{print $NF}'
)
echo $TOKEN_OAUTH
}

standard_login(){
# $1: username
# $2: password
TOKEN=$(curl --silent -X POST \
http://127.0.0.1:8001/auth/token/login/ \
-H "Content-Type: application/x-www-form-urlencoded" \
-d "username=$1&password=$2" | jq -r ".access_token"
)
echo $TOKEN
}

assert_users_and_oauth() {
# $1 desired number of users
# $2 desired number of oauth accounts
USERS=$(sqlite3 $SQLITE_PATH "SELECT * FROM user_oauth;" | wc -l)
OAUTH_ACCOUNTS=$(sqlite3 $SQLITE_PATH "SELECT * FROM oauthaccount;" | wc -l)
if [ "$USERS" -ne "$1" ] || [ "$OAUTH_ACCOUNTS" -ne "$2" ]; then
exit 1
fi
}

assert_email_and_id(){
# $1 access token
# $2 desired email
# $3 desired user id
USER=$(
curl --silent -H "Authorization: Bearer $1" \
http://127.0.0.1:8001/auth/current-user/
)
EMAIL=$(echo $USER | jq -r ".email")
ID=$(echo $USER | jq -r ".id")
if [ "$EMAIL" != "$2" ]; then
exit 1
fi
if [ "$ID" != "$3" ]; then
exit 1
fi
}

# --- Test


# Register "kilgore@kilgore.trout" (the user from Dex) as regular account.
SUPERUSER_TOKEN=$(standard_login "admin@fractal.xy" "1234")

assert_users_and_oauth 1 0
curl -X POST \
http://127.0.0.1:8001/auth/register/ \
-H "Content-Type: application/json" \
-H "Authorization: Bearer $SUPERUSER_TOKEN" \
-d '{"email": "kilgore@kilgore.trout", "password": "kilgore"}'
assert_users_and_oauth 2 0

WHOAMI=$(
curl --silent -H "Authorization: Bearer $TOKEN" \
http://127.0.0.1:8001/auth/current-user/
# Login with "kilgore@kilgore.trout" with standard login.
USER_TOKEN=$(standard_login "kilgore@kilgore.trout" "kilgore")
USER_ID=$(
curl --silent -H "Authorization: Bearer $USER_TOKEN" \
http://127.0.0.1:8001/auth/current-user/ | jq -r ".id"
)

EXPECTED_USER='{"id":2,"email":"kilgore@kilgore.trout","is_active":true,"is_superuser":false,"is_verified":false,"slurm_user":null,"cache_dir":null,"username":null,"slurm_accounts":[],"group_ids":null,"group_names":null}'
assert_email_and_id $USER_TOKEN "kilgore@kilgore.trout" $USER_ID

# First oauth login:
# - create "kilgore@kilgore.trout" oauth account,
# - associate by email with existing user.
USER_TOKEN_OAUTH=$(oauth_login)

assert_users_and_oauth 2 1
assert_email_and_id $USER_TOKEN_OAUTH "kilgore@kilgore.trout" $USER_ID

# Change email into "kilgore@fractal.xy".
curl -X PATCH \
"http://127.0.0.1:8001/auth/users/$USER_ID/" \
-H "Content-Type: application/json" \
-H "Authorization: Bearer $SUPERUSER_TOKEN" \
-d '{"email": "kilgore@fractal.xy"}'

# Test I can login as "kilgore@fractal.xy" with both standard and oauth login.
USER_TOKEN=$(standard_login "kilgore@fractal.xy" "kilgore")
assert_email_and_id $USER_TOKEN "kilgore@fractal.xy" $USER_ID

USER_TOKEN_OAUTH=$(oauth_login)
assert_email_and_id $USER_TOKEN_OAUTH "kilgore@fractal.xy" $USER_ID

# Remove all oauth accounts from db.
assert_users_and_oauth 2 1
sqlite3 $SQLITE_PATH "DELETE FROM oauthaccount;"
assert_users_and_oauth 2 0

# Test I can login as "kilgore@fractal.xy" with standard login.
USER_TOKEN=$(standard_login "kilgore@fractal.xy" "kilgore")
assert_email_and_id $USER_TOKEN "kilgore@fractal.xy" $USER_ID

# Using oauth login creates another user: "kilgore@kilgore.trout".
assert_users_and_oauth 2 0
USER_TOKEN_OAUTH=$(oauth_login)
assert_users_and_oauth 3 1

# Assert that WHOAMI is equal to EXPECTED_USER
diff <(echo $WHOAMI | jq --sort-keys .) <(echo $EXPECTED_USER | jq --sort-keys .)
if [ "$?" != "0" ]; then
exit 1
fi
assert_email_and_id $USER_TOKEN_OAUTH "kilgore@kilgore.trout" $((USER_ID+1))
2 changes: 1 addition & 1 deletion scripts/validate_db_data_with_read_schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
stm = select(UserOAuth)
users = db.execute(stm).scalars().unique().all()
for user in sorted(users, key=lambda x: x.id):
UserRead(**user.model_dump())
UserRead(**user.model_dump(), oauth_accounts=user.oauth_accounts)
print(f"User {user.id} validated")

# USER GROUPS
Expand Down
Loading

0 comments on commit 8cb2201

Please sign in to comment.