Skip to content

Commit

Permalink
fix: Refactor in order to include source of token issuing
Browse files Browse the repository at this point in the history
  • Loading branch information
Paula-Kli committed Oct 11, 2023
1 parent a30f295 commit 8898475
Show file tree
Hide file tree
Showing 36 changed files with 565 additions and 376 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"""Add basic auth token
Revision ID: c9f30ccd4650
Revises: d8cf851562cd
Revises: 1a4208c18909
Create Date: 2023-09-06 14:42:53.016924
"""
Expand All @@ -13,24 +13,21 @@

# revision identifiers, used by Alembic.
revision = "c9f30ccd4650"
down_revision = "d8cf851562cd"
down_revision = "1a4208c18909"
branch_labels = None
depends_on = None


def upgrade():
# ### commands auto generated by Alembic - please adjust! ###
op.create_table(
"basic_auth_token",
sa.Column("id", sa.Integer(), autoincrement=True, nullable=False),
sa.Column("user_id", sa.Integer(), nullable=False),
sa.Column("hash", sa.String(), nullable=False),
sa.Column("expiration_date", sa.Date(), nullable=False),
sa.Column("description", sa.String(), nullable=False),
sa.ForeignKeyConstraint(
["user_id"],
["users.id"],
),
sa.Column("source", sa.String(), nullable=False),
sa.ForeignKeyConstraint(["user_id"], ["users.id"], ondelete="CASCADE"),
sa.PrimaryKeyConstraint("id"),
)
op.create_index(
Expand All @@ -39,4 +36,3 @@ def upgrade():
["id"],
unique=False,
)
# ### end Alembic commands ###
73 changes: 36 additions & 37 deletions backend/capellacollab/core/authentication/basic_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,54 +17,53 @@
class HTTPBasicAuth(security.HTTPBasic):
async def __call__( # type: ignore
self, request: fastapi.Request
) -> tuple[str | None, fastapi.HTTPException | None]:
) -> str | None:
credentials: security.HTTPBasicCredentials | None = (
await super().__call__(request)
)
if not credentials:
error = fastapi.HTTPException(
status_code=status.HTTP_401_UNAUTHORIZED,
detail="Not authenticated",
headers={"WWW-Authenticate": "Bearer, Basic"},
)
if self.auto_error:
raise error
return None, error
raise fastapi.HTTPException(

Check warning on line 26 in backend/capellacollab/core/authentication/basic_auth.py

View check run for this annotation

Codecov / codecov/patch

backend/capellacollab/core/authentication/basic_auth.py#L26

Added line #L26 was not covered by tests
status_code=status.HTTP_401_UNAUTHORIZED,
detail="Not authenticated",
headers={"WWW-Authenticate": "Bearer, Basic"},
)
return None

Check warning on line 31 in backend/capellacollab/core/authentication/basic_auth.py

View check run for this annotation

Codecov / codecov/patch

backend/capellacollab/core/authentication/basic_auth.py#L31

Added line #L31 was not covered by tests
with database.SessionLocal() as session:
user = user_crud.get_user_by_name(session, credentials.username)
token_data = None
if user:
token_data = token_crud.get_token(
db_token = (
token_crud.get_token_by_token_and_user(
session, credentials.password, user.id
)
if not token_data or not user or token_data.user_id != user.id:
logger.error("Token invalid for user %s", credentials.username)
error = fastapi.HTTPException(
status_code=status.HTTP_401_UNAUTHORIZED,
detail={
"err_code": "TOKEN_INVALID",
"reason": "The used token is not valid.",
},
headers={"WWW-Authenticate": "Bearer, Basic"},
)
if user
else None
)
if not db_token:
logger.info("Token invalid for user %s", credentials.username)
if self.auto_error:
raise error
return None, error
raise fastapi.HTTPException(
status_code=status.HTTP_401_UNAUTHORIZED,
detail={
"err_code": "BASIC_TOKEN_INVALID",
"reason": "The used token is not valid.",
},
headers={"WWW-Authenticate": "Bearer, Basic"},
)
return None

Check warning on line 52 in backend/capellacollab/core/authentication/basic_auth.py

View check run for this annotation

Codecov / codecov/patch

backend/capellacollab/core/authentication/basic_auth.py#L52

Added line #L52 was not covered by tests

if token_data.expiration_date < datetime.date.today():
logger.error("Token expired for user %s", credentials.username)
error = fastapi.HTTPException(
status_code=status.HTTP_401_UNAUTHORIZED,
detail={
"err_code": "token_exp",
"reason": "The Signature of the token is expired. Please request a new access token.",
},
headers={"WWW-Authenticate": "Bearer, Basic"},
)
if db_token.expiration_date < datetime.date.today():
logger.info("Token expired for user %s", credentials.username)

Check warning on line 55 in backend/capellacollab/core/authentication/basic_auth.py

View check run for this annotation

Codecov / codecov/patch

backend/capellacollab/core/authentication/basic_auth.py#L55

Added line #L55 was not covered by tests
if self.auto_error:
raise error
return None, error
return self.get_username(credentials), None
raise fastapi.HTTPException(

Check warning on line 57 in backend/capellacollab/core/authentication/basic_auth.py

View check run for this annotation

Codecov / codecov/patch

backend/capellacollab/core/authentication/basic_auth.py#L57

Added line #L57 was not covered by tests
status_code=status.HTTP_401_UNAUTHORIZED,
detail={
"err_code": "token_exp",
"reason": "The Signature of the token is expired. Please request a new access token.",
},
headers={"WWW-Authenticate": "Bearer, Basic"},
)
return None
return self.get_username(credentials)

Check warning on line 66 in backend/capellacollab/core/authentication/basic_auth.py

View check run for this annotation

Codecov / codecov/patch

backend/capellacollab/core/authentication/basic_auth.py#L65-L66

Added lines #L65 - L66 were not covered by tests

def get_username(self, credentials: security.HTTPBasicCredentials) -> str:
return credentials.model_dump()["username"]
return credentials.username

Check warning on line 69 in backend/capellacollab/core/authentication/basic_auth.py

View check run for this annotation

Codecov / codecov/patch

backend/capellacollab/core/authentication/basic_auth.py#L69

Added line #L69 was not covered by tests
50 changes: 24 additions & 26 deletions backend/capellacollab/core/authentication/injectables.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
from fastapi import status
from fastapi.openapi import models as openapi_models
from fastapi.security import base as security_base
from fastapi.security import utils as security_utils
from sqlalchemy import orm

from capellacollab.core import database
from capellacollab.core.authentication import basic_auth, jwt_bearer
Expand Down Expand Up @@ -70,28 +72,24 @@ async def get_username(
request: fastapi.Request,
_unused1=fastapi.Depends(OpenAPIPersonalAccessToken()),
_unused2=fastapi.Depends(OpenAPIBearerToken()),
):
basic = await basic_auth.HTTPBasicAuth(auto_error=False)(request)
jwt = await jwt_bearer.JWTBearer(auto_error=False)(request)

jwt_username, jwt_error = jwt
if jwt_username:
return jwt_username

basic_username, basic_error = basic
if basic_username:
return basic_username

if jwt_error:
raise jwt_error
if basic_error:
raise basic_error

raise fastapi.HTTPException(
status_code=status.HTTP_401_UNAUTHORIZED,
detail="Token is none and username cannot be derived",
headers={"WWW-Authenticate": "Bearer, Basic"},
)
) -> str:
authorization = request.headers.get("Authorization")
scheme, _ = security_utils.get_authorization_scheme_param(authorization)
username = None
match scheme.lower():
case "basic":
username = await basic_auth.HTTPBasicAuth()(request)
case "bearer":
username = await jwt_bearer.JWTBearer()(request)
case _:
raise fastapi.HTTPException(

Check warning on line 85 in backend/capellacollab/core/authentication/injectables.py

View check run for this annotation

Codecov / codecov/patch

backend/capellacollab/core/authentication/injectables.py#L84-L85

Added lines #L84 - L85 were not covered by tests
status_code=status.HTTP_401_UNAUTHORIZED,
detail="Token is none and username cannot be derived",
headers={"WWW-Authenticate": "Bearer, Basic"},
)

assert username
return username


class RoleVerification:
Expand All @@ -101,8 +99,8 @@ def __init__(self, required_role: users_models.Role, verify: bool = True):

def __call__(
self,
username=fastapi.Depends(get_username),
db=fastapi.Depends(database.get_db),
username: str = fastapi.Depends(get_username),
db: orm.Session = fastapi.Depends(database.get_db),
) -> bool:
if not (user := users_crud.get_user_by_name(db, username)):
if self.verify:
Expand Down Expand Up @@ -148,8 +146,8 @@ def __init__(
def __call__(
self,
project_slug: str,
username=fastapi.Depends(get_username),
db=fastapi.Depends(database.get_db),
username: str = fastapi.Depends(get_username),
db: orm.Session = fastapi.Depends(database.get_db),
) -> bool:
if not (user := users_crud.get_user_by_name(db, username)):
if self.verify:
Expand Down
32 changes: 15 additions & 17 deletions backend/capellacollab/core/authentication/jwt_bearer.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,31 +27,29 @@ def __init__(self, auto_error: bool = True):

async def __call__( # type: ignore
self, request: fastapi.Request
) -> tuple[str | None, fastapi.HTTPException | None]:
) -> str | None:
credentials: security.HTTPAuthorizationCredentials | None = (
await super().__call__(request)
)

if not credentials or credentials.scheme != "Bearer":
error = fastapi.HTTPException(
status_code=status.HTTP_401_UNAUTHORIZED,
detail="Not authenticated",
headers={"WWW-Authenticate": "Bearer, Basic"},
)
if self.auto_error:
raise error
return None, error
raise fastapi.HTTPException(
status_code=status.HTTP_401_UNAUTHORIZED,
detail="Not authenticated",
headers={"WWW-Authenticate": "Bearer, Basic"},
)
return None
if token_decoded := self.validate_token(credentials.credentials):
self.initialize_user(token_decoded)
return self.get_username(token_decoded), None
error = fastapi.HTTPException(
status_code=status.HTTP_401_UNAUTHORIZED,
detail="Not authenticated",
headers={"WWW-Authenticate": "Bearer, Basic"},
)
return self.get_username(token_decoded)

Check warning on line 45 in backend/capellacollab/core/authentication/jwt_bearer.py

View check run for this annotation

Codecov / codecov/patch

backend/capellacollab/core/authentication/jwt_bearer.py#L45

Added line #L45 was not covered by tests
if self.auto_error:
raise error
return None, error
raise fastapi.HTTPException(

Check warning on line 47 in backend/capellacollab/core/authentication/jwt_bearer.py

View check run for this annotation

Codecov / codecov/patch

backend/capellacollab/core/authentication/jwt_bearer.py#L47

Added line #L47 was not covered by tests
status_code=status.HTTP_401_UNAUTHORIZED,
detail="Not authenticated",
headers={"WWW-Authenticate": "Bearer, Basic"},
)
return None

def get_username(self, token_decoded: dict[str, str]) -> str:
return token_decoded[

Check warning on line 55 in backend/capellacollab/core/authentication/jwt_bearer.py

View check run for this annotation

Codecov / codecov/patch

backend/capellacollab/core/authentication/jwt_bearer.py#L55

Added line #L55 was not covered by tests
Expand All @@ -74,7 +72,7 @@ def validate_token(self, token: str) -> dict[str, t.Any] | None:
raise fastapi.HTTPException(
status_code=status.HTTP_401_UNAUTHORIZED,
detail={
"err_code": "TOKEN_INVALID",
"err_code": "JWT_TOKEN_INVALID",
"reason": "The used token is not valid.",
},
) from None
Expand Down
2 changes: 1 addition & 1 deletion backend/capellacollab/projects/routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ def get_projects(
users_injectables.get_own_user
),
db: orm.Session = fastapi.Depends(database.get_db),
username=fastapi.Depends(auth_injectables.get_username),
username: str = fastapi.Depends(auth_injectables.get_username),
log: logging.LoggerAdapter = fastapi.Depends(
core_logging.get_request_logger
),
Expand Down
4 changes: 2 additions & 2 deletions backend/capellacollab/projects/toolmodels/backups/routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ def create_backup(
toolmodels_injectables.get_existing_capella_model
),
db: orm.Session = fastapi.Depends(database.get_db),
username=fastapi.Depends(auth_injectables.get_username),
username: str = fastapi.Depends(auth_injectables.get_username),
):
git_model = git_injectables.get_existing_git_model(
body.git_model_id, capella_model, db
Expand Down Expand Up @@ -141,7 +141,7 @@ def delete_pipeline(
injectables.get_existing_pipeline
),
db: orm.Session = fastapi.Depends(database.get_db),
username=fastapi.Depends(auth_injectables.get_username),
username: str = fastapi.Depends(auth_injectables.get_username),
force: bool = False,
):
try:
Expand Down
2 changes: 1 addition & 1 deletion backend/capellacollab/projects/users/routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ def get_current_user(
projects_injectables.get_existing_project
),
db: orm.Session = fastapi.Depends(database.get_db),
username=fastapi.Depends(auth_injectables.get_username),
username: str = fastapi.Depends(auth_injectables.get_username),
) -> models.ProjectUserAssociation | models.ProjectUser:
if auth_injectables.RoleVerification(
required_role=users_models.Role.ADMIN, verify=False
Expand Down
1 change: 0 additions & 1 deletion backend/capellacollab/sessions/hooks/interface.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ def configuration_hook(
user: users_models.DatabaseUser,
tool_version: tools_models.DatabaseVersion,
tool: tools_models.DatabaseTool,
username: str,
**kwargs,
) -> tuple[
dict[str, str],
Expand Down
3 changes: 1 addition & 2 deletions backend/capellacollab/sessions/hooks/jupyter.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ def configuration_hook( # type: ignore[override]
db: orm.Session,
user: users_models.DatabaseUser,
tool: tools_models.DatabaseTool,
username: str,
**kwargs,
) -> tuple[
JupyterConfigEnvironment,
Expand All @@ -63,7 +62,7 @@ def configuration_hook( # type: ignore[override]
"CSP_ORIGIN_HOST": f"{self._general_conf.get('scheme')}://{self._general_conf.get('host')}:{self._general_conf.get('port')}",
}

volumes = self._get_project_share_volume_mounts(db, username, tool)
volumes = self._get_project_share_volume_mounts(db, user.name, tool)

return environment, volumes, [] # type: ignore[return-value]

Expand Down
2 changes: 1 addition & 1 deletion backend/capellacollab/sessions/injectables.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
def get_existing_session(
session_id: str,
db: orm.Session = fastapi.Depends(database.get_db),
username=fastapi.Depends(auth_injectables.get_username),
username: str = fastapi.Depends(auth_injectables.get_username),
) -> models.DatabaseSession:
if not (session := crud.get_session_by_id(db, session_id)):
raise fastapi.HTTPException(
Expand Down
6 changes: 3 additions & 3 deletions backend/capellacollab/sessions/routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ def get_current_sessions(
users_injectables.get_own_user
),
db: orm.Session = fastapi.Depends(database.get_db),
username=fastapi.Depends(auth_injectables.get_username),
username: str = fastapi.Depends(auth_injectables.get_username),
):
if auth_injectables.RoleVerification(
required_role=users_models.Role.ADMIN, verify=False
Expand Down Expand Up @@ -283,7 +283,7 @@ def request_persistent_session(
),
db: orm.Session = fastapi.Depends(database.get_db),
operator: k8s.KubernetesOperator = fastapi.Depends(operators.get_operator),
username=fastapi.Depends(auth_injectables.get_username),
username: str = fastapi.Depends(auth_injectables.get_username),
):
log.info("Starting persistent session for user %s", user.name)

Expand Down Expand Up @@ -604,7 +604,7 @@ def get_sessions_for_user(
users_injectables.get_own_user
),
db: orm.Session = fastapi.Depends(database.get_db),
username=fastapi.Depends(auth_injectables.get_username),
username: str = fastapi.Depends(auth_injectables.get_username),
):
if user != current_user and not auth_injectables.RoleVerification(
required_role=users_models.Role.ADMIN, verify=False
Expand Down
4 changes: 2 additions & 2 deletions backend/capellacollab/users/injectables.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

def get_own_user(
db: orm.Session = fastapi.Depends(database.get_db),
username=fastapi.Depends(auth_injectables.get_username),
username: str = fastapi.Depends(auth_injectables.get_username),
) -> models.DatabaseUser:
if user := crud.get_user_by_name(db, username):
return user
Expand All @@ -25,7 +25,7 @@ def get_own_user(
def get_existing_user(
user_id: int | t.Literal["current"],
db=fastapi.Depends(database.get_db),
username=fastapi.Depends(auth_injectables.get_username),
username: str = fastapi.Depends(auth_injectables.get_username),
) -> models.DatabaseUser:
if user_id == "current":
return get_own_user(db, username)

Check warning on line 31 in backend/capellacollab/users/injectables.py

View check run for this annotation

Codecov / codecov/patch

backend/capellacollab/users/injectables.py#L31

Added line #L31 was not covered by tests
Expand Down
5 changes: 5 additions & 0 deletions backend/capellacollab/users/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
from capellacollab.projects.users.models import ProjectUserAssociation
from capellacollab.sessions.models import DatabaseSession
from capellacollab.users.events.models import DatabaseUserHistoryEvent
from capellacollab.users.tokens.models import DatabaseUserToken


class Role(enum.Enum):
Expand Down Expand Up @@ -64,3 +65,7 @@ class DatabaseUser(database.Base):
events: orm.Mapped[list[DatabaseUserHistoryEvent]] = orm.relationship(
back_populates="user", foreign_keys="DatabaseUserHistoryEvent.user_id"
)

tokens: orm.Mapped[list[DatabaseUserToken]] = orm.relationship(
back_populates="user", cascade="all, delete-orphan"
)
Loading

0 comments on commit 8898475

Please sign in to comment.