Skip to content

Commit

Permalink
Blocks expired user
Browse files Browse the repository at this point in the history
  • Loading branch information
pcrespov committed Sep 19, 2022
1 parent e6e9bc4 commit 6c2142b
Show file tree
Hide file tree
Showing 7 changed files with 72 additions and 43 deletions.
46 changes: 27 additions & 19 deletions services/web/server/src/simcore_service_webserver/login/handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
BANNED,
CHANGE_EMAIL,
CONFIRMATION_PENDING,
EXPIRED,
REGISTRATION,
RESET_PASSWORD,
USER,
Expand All @@ -56,6 +57,28 @@ def _get_user_name(email: str) -> str:
return username


def _validate_user_status(user: dict, cfg):
user_status: str = user["status"]

if user_status == BANNED or user["role"] == ANONYMOUS:
raise web.HTTPUnauthorized(
reason=cfg.MSG_USER_BANNED, content_type=MIMETYPE_APPLICATION_JSON
) # 401

if user_status == EXPIRED:
raise web.HTTPUnauthorized(
reason=cfg.MSG_USER_EXPIRED, content_type=MIMETYPE_APPLICATION_JSON
) # 401

if user_status == CONFIRMATION_PENDING:
raise web.HTTPUnauthorized(
reason=cfg.MSG_ACTIVATION_REQUIRED,
content_type=MIMETYPE_APPLICATION_JSON,
) # 401

assert user_status == ACTIVE # nosec


async def register(request: web.Request):
"""
Starts user's registration by providing an email, password and
Expand Down Expand Up @@ -268,26 +291,19 @@ async def login(request: web.Request):
password = body.password

user = await db.get_user({"email": email})

if not user:
raise web.HTTPUnauthorized(
reason=cfg.MSG_UNKNOWN_EMAIL, content_type=MIMETYPE_APPLICATION_JSON
)

if user["status"] == BANNED or user["role"] == ANONYMOUS:
raise web.HTTPUnauthorized(
reason=cfg.MSG_USER_BANNED, content_type=MIMETYPE_APPLICATION_JSON
)
_validate_user_status(user, cfg)

if not check_password(password, user["password_hash"]):
raise web.HTTPUnauthorized(
reason=cfg.MSG_WRONG_PASSWORD, content_type=MIMETYPE_APPLICATION_JSON
)

if user["status"] == CONFIRMATION_PENDING:
raise web.HTTPUnauthorized(
reason=cfg.MSG_ACTIVATION_REQUIRED, content_type=MIMETYPE_APPLICATION_JSON
)

assert user["status"] == ACTIVE, "db corrupted. Invalid status" # nosec
assert user["email"] == email, "db corrupted. Invalid email" # nosec

Expand Down Expand Up @@ -439,16 +455,7 @@ async def reset_password(request: web.Request):
reason=cfg.MSG_UNKNOWN_EMAIL, content_type=MIMETYPE_APPLICATION_JSON
) # 422

if user["status"] == BANNED:
raise web.HTTPUnauthorized(
reason=cfg.MSG_USER_BANNED, content_type=MIMETYPE_APPLICATION_JSON
) # 401

if user["status"] == CONFIRMATION_PENDING:
raise web.HTTPUnauthorized(
reason=cfg.MSG_ACTIVATION_REQUIRED,
content_type=MIMETYPE_APPLICATION_JSON,
) # 401
_validate_user_status(user, cfg)

assert user["status"] == ACTIVE # nosec
assert user["email"] == email # nosec
Expand All @@ -458,6 +465,7 @@ async def reset_password(request: web.Request):
reason=cfg.MSG_OFTEN_RESET_PASSWORD,
content_type=MIMETYPE_APPLICATION_JSON,
) # 401

except web.HTTPError as err:
# Email wiht be an explanation and suggest alternative approaches or ways to contact support for help
try:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,8 @@ def get_confirmation_lifetime(
MSG_UNKNOWN_EMAIL: str = "This email is not registered"
MSG_WRONG_PASSWORD: str = "Wrong password"
MSG_PASSWORD_MISMATCH: str = "Password and confirmation do not match"
MSG_USER_BANNED: str = "This user is banned"
MSG_USER_BANNED: str = "This user does not have anymore access"
MSG_USER_EXPIRED: str = "This account has expired and does not have anymore access. Please contact support for further details."
MSG_ACTIVATION_REQUIRED: str = (
"You have to activate your account via email, before you can login"
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,14 @@ def _to_names(enum_cls, names):
return [getattr(enum_cls, att).name for att in names.split()]


CONFIRMATION_PENDING, ACTIVE, BANNED = _to_names(
UserStatus, "CONFIRMATION_PENDING ACTIVE BANNED"
CONFIRMATION_PENDING, ACTIVE, BANNED, EXPIRED = (
UserStatus.CONFIRMATION_PENDING.name,
UserStatus.ACTIVE.name,
UserStatus.BANNED.name,
UserStatus.EXPIRED.name,
)
assert len(UserStatus) == 4 # nosec


ANONYMOUS, GUEST, USER, TESTER = _to_names(UserRole, "ANONYMOUS GUEST USER TESTER")

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
import logging

import passlib.hash
import sqlalchemy as sa
from aiohttp import web
from aiohttp_security.api import (
AUTZ_KEY,
Expand All @@ -27,7 +26,9 @@
async def check_credentials(engine: Engine, email: str, password: str) -> bool:
async with engine.acquire() as conn:
query = users.select().where(
sa.and_(users.c.email == email, users.c.status != UserStatus.BANNED)
(users.c.email == email)
& (users.c.status != UserStatus.BANNED)
& (users.c.status != UserStatus.EXPIRED)
)
ret = await conn.execute(query)
user = await ret.fetchone()
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import logging
from typing import Dict, Optional, Tuple, Union
from typing import Optional, Union

import attr
import sqlalchemy as sa
from aiohttp import web
from aiohttp_security.abc import AbstractAuthorizationPolicy
from aiopg.sa import Engine
Expand Down Expand Up @@ -37,18 +36,21 @@ def engine(self) -> Engine:
return self.app[APP_DB_ENGINE_KEY]

@retry(**PostgresRetryPolicyUponOperation(log).kwargs)
async def _pg_query_user(self, identity: str) -> RowProxy:
async def _pg_query_user(self, identity: str) -> Optional[RowProxy]:
# NOTE: Keeps a cache for a few seconds. Observed successive streams of this query
row = self.timed_cache.get(identity)
if not row:
if row is None:
query = users.select().where(
sa.and_(users.c.email == identity, users.c.status != UserStatus.BANNED)
(users.c.email == identity)
& (users.c.status != UserStatus.BANNED)
& (users.c.status != UserStatus.EXPIRED)
)
async with self.engine.acquire() as conn:
# NOTE: sometimes it raises psycopg2.DatabaseError in #880 and #1160
ret: ResultProxy = await conn.execute(query)
row: RowProxy = await ret.fetchone()
self.timed_cache[identity] = row
res: ResultProxy = await conn.execute(query)
row: Optional[RowProxy] = await res.fetchone()
if row is not None:
self.timed_cache[identity] = row
return row

async def authorized_userid(self, identity: str) -> Optional[int]:
Expand All @@ -64,8 +66,8 @@ async def authorized_userid(self, identity: str) -> Optional[int]:
async def permits(
self,
identity: str,
permission: Union[str, Tuple],
context: Optional[Dict] = None,
permission: Union[str, tuple],
context: Optional[dict] = None,
) -> bool:
"""Determines whether an identified user has permission
Expand Down
13 changes: 9 additions & 4 deletions services/web/server/tests/unit/with_dbs/01/test_login_login.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,21 +76,26 @@ async def test_login_with_wrong_password(
assert login_options.MSG_WRONG_PASSWORD in await r.text()


async def test_login_banned_user(client: TestClient, login_options: LoginOptions):
@pytest.mark.parametrize("user_status", (UserStatus.BANNED, UserStatus.EXPIRED))
async def test_login_blocked_user(
client: TestClient, login_options: LoginOptions, user_status: UserStatus
):
expected_msg: str = getattr(login_options, f"MSG_USER_{user_status.name.upper()}")

assert client.app
url = client.app.router["auth_login"].url_for()
r = await client.post(f"{url}")
assert login_options.MSG_USER_BANNED not in await r.text()
assert expected_msg not in await r.text()

async with NewUser({"status": UserStatus.BANNED.name}, app=client.app) as user:
async with NewUser({"status": user_status.name}, app=client.app) as user:
r = await client.post(
f"{url}", json={"email": user["email"], "password": user["raw_password"]}
)
payload = await r.json()

assert r.status == web.HTTPUnauthorized.status_code, str(payload)
assert r.url.path == url.path
assert login_options.MSG_USER_BANNED in payload["error"]["errors"][0]["message"]
assert expected_msg in payload["error"]["errors"][0]["message"]


async def test_login_inactive_user(client: TestClient, login_options: LoginOptions):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,12 +65,18 @@ async def test_unknown_email(
assert parse_test_marks(out)["reason"] == cfg.MSG_UNKNOWN_EMAIL


async def test_banned_user(client: TestClient, cfg: LoginOptions, capsys):
@pytest.mark.parametrize("user_status", (UserStatus.BANNED, UserStatus.EXPIRED))
async def test_blocked_user(
client: TestClient, cfg: LoginOptions, capsys, user_status: UserStatus
):
assert client.app
expected_msg = getattr(cfg, f"MSG_USER_{user_status.name.upper()}")

reset_url = client.app.router["auth_reset_password"].url_for()

async with NewUser({"status": UserStatus.BANNED.name}, app=client.app) as user:
async with NewUser({"status": user_status.name}, app=client.app) as user:
rp = await client.post(
reset_url,
f"{reset_url}",
json={
"email": user["email"],
},
Expand All @@ -79,11 +85,12 @@ async def test_banned_user(client: TestClient, cfg: LoginOptions, capsys):
assert rp.url.path == reset_url.path
await assert_status(rp, web.HTTPOk, cfg.MSG_EMAIL_SENT.format(**user))

out, err = capsys.readouterr()
assert parse_test_marks(out)["reason"] == cfg.MSG_USER_BANNED
out, _ = capsys.readouterr()
assert parse_test_marks(out)["reason"] == expected_msg


async def test_inactive_user(client: TestClient, cfg: LoginOptions, capsys):
assert client.app
reset_url = client.app.router["auth_reset_password"].url_for()

async with NewUser(
Expand Down

0 comments on commit 6c2142b

Please sign in to comment.