[16.0] pms_fastapi, pms_api_rest: second factor on login

[jesusVMayor]

What

An account with a second factor enabled could not use this API at all: the
new login asked for a credential such an account is not allowed to use, and
the legacy one let it in on its password alone, so the second factor
protected nothing there.

• POST /pmsApi/login now answers /errors/mfa-required and starts a
  challenge, unless the device was remembered on a previous login.
• POST /pmsApi/login/mfa takes the code and finishes the login, optionally
  remembering the device.
• The legacy login refuses these accounts instead of bypassing their second
  factor. API keys keep working there, and accounts without a second factor
  are unaffected.

The challenge

A row with the user it belongs to, an expiry and an attempt counter, handed
to the client as a cookie it never reads. It grants nothing on its own: the
code is still required. Attempts are counted against it, so a wrong code
costs the caller its challenge and not its account, and once they run out
the login starts over from the password.

Errors are problem+json with a stable type, which is what the client
branches on. They are returned rather than raised on purpose: raising rolls
the transaction back, and with it the attempt just spent.

A remembered device is the same trusted device the backoffice keeps, under a
scope of its own so that remembering here does not overwrite what was
remembered there, nor the other way round.

Also fixed here

The refresh token carried nothing of its own, so its value was a pure
function of the second it expires at. Two logins of the same user within the
same second produced the same string and the second one hit the uniqueness
constraint — an unhandled database error, so a 500 and no session at all.
Rare until now, unavoidable once a login can be followed by another one,
which is what remembering a device does.

Testing

Seven tests over the whole flow with the HTTP client, plus two for the
legacy refusal. Full suites of both modules green: 264 tests on the FastAPI
side, and on the legacy one the only failures are a pre-existing class whose
fixtures need a field no longer present in a minimal install.

[github-actions]

Diff Coverage

Diff: origin/16.0...HEAD, staged and unstaged changes

• pms_api_rest/services/pms_login_service.py (0.0%): Missing lines 50
• pms_fastapi/models/pms_mfa_challenge.py (0.0%): Missing lines 1,4,13-14,16,19-21,23,27-30
• pms_fastapi/routers/login.py (0.0%): Missing lines 1-3,5-6,8,13,15-20,22,24,45,53,56,64-65,70,76,83,112-113,119-120,131,133-138,141,146,148-150,157,162-163,165-173,176-180,185-190,192-193,199-203,207-208,216,219,225,227,229,231-232,235,279
• pms_fastapi/schemas/pms_login.py (0.0%): Missing lines 11-13
• roomdoo_fastapi/routers/login.py (0.0%): Missing lines 1,71

Summary

• Total: 99 lines
• Missing: 99 lines
• Coverage: 0%

pms_api_rest/services/pms_login_service.py

Lines 46-54

  46             # This login cannot ask for a second factor, so it does not promise
  47             # to: an account protected by one is refused here and has to use
  48             # the login that can. An API key stays valid, which is the
  49             # credential meant for callers with nobody in front of them.
! 50             user_record.with_user(user_record)._check_credentials(
  51                 user.password, {"interactive": False}
  52             )
  53         except AccessDenied as e:
  54             raise werkzeug.exceptions.Unauthorized(_("wrong user/pass")) from e

---

pms_fastapi/models/pms_mfa_challenge.py

Lines 1-8

! 1 from odoo import api, fields, models
  2 
  3 
! 4 class PmsFastapiMfaChallenge(models.Model):
  5     """A login that passed the password and still owes its second factor.
  6 
  7     It is the API's equivalent of the pre-authenticated session the web login
  8     keeps: on its own it grants nothing, it only says which user is allowed to

---

Lines 9-31

   9     hand in a code. Attempts are counted here so that a wrong code costs the
  10     caller its challenge instead of its account.
  11     """
  12 
! 13     _name = "pms.fastapi.mfa.challenge"
! 14     _description = "Pending second factor"
  15 
! 16     user_id = fields.Many2one(
  17         "res.users", required=True, index=True, ondelete="cascade"
  18     )
! 19     token = fields.Char(required=True, index=True)
! 20     expire = fields.Datetime(required=True)
! 21     attempts = fields.Integer(default=0)
  22 
! 23     _sql_constraints = [
  24         ("unique_token", "unique(token)", "The token must be unique!"),
  25     ]
  26 
! 27     @api.autovacuum
! 28     def _remove_expired_challenges(self):
! 29         records = self.search([("expire", "<", fields.Datetime.now())])
! 30         return records.unlink()

---

pms_fastapi/routers/login.py

Lines 1-28

!  1 import re
!  2 import secrets
!  3 from datetime import timedelta
   4 
!  5 from fastapi import Request, Response, status
!  6 from fastapi.responses import JSONResponse
   7 
!  8 from odoo import _, fields, models
   9 from odoo.exceptions import AccessDenied
  10 
  11 from odoo.addons.pms_fastapi.dependencies import PublicEnv
  12 from odoo.addons.pms_fastapi.models.fastapi_endpoint import pms_api_router
! 13 from odoo.addons.pms_fastapi.schemas.pms_login import PmsLoginInput, PmsMfaInput
  14 
! 15 MFA_COOKIE = "mfa"
! 16 MFA_MAX_AGE = 5 * 60
! 17 MFA_MAX_ATTEMPTS = 5
! 18 DEVICE_COOKIE = "pms_td"
! 19 DEVICE_SCOPE = "api"
! 20 DEVICE_MAX_AGE = 90 * 86400
  21 # The only second factor this API knows how to ask for.
! 22 SUPPORTED_MFA_TYPE = "totp"
  23 
! 24 UNAUTHORIZED_RESPONSE = {
  25     "description": "Unauthorized",
  26     "content": {
  27         "application/problem+json": {
  28             "example": {

---

Lines 41-49

  41     status_code=204,
  42     responses={401: UNAUTHORIZED_RESPONSE, 204: {"model": None}},
  43     tags=["login"],
  44 )
! 45 async def login(user: PmsLoginInput, request: Request, env: PublicEnv):
  46     """
  47     Starts a session.
  48 
  49     When the account is protected by a second factor the answer is

---

Lines 49-60

  49     When the account is protected by a second factor the answer is
  50     `/errors/mfa-required` and the code has to be sent to `/login/mfa`, unless
  51     this device was remembered on a previous login.
  52     """
! 53     endpoint = env["pms.fastapi.login.endpoint"]
  54     user_record = env["res.users"].sudo().search([("login", "=", user.username)])
  55     if not user_record:
! 56         return endpoint._invalid_credentials_response()
  57     try:
  58         # A person is waiting on the other side, so a password is the expected
  59         # credential here; the second factor is asked for below.
  60         user_record.with_user(user_record)._check_credentials(

---

Lines 60-74

  60         user_record.with_user(user_record)._check_credentials(
  61             user.password.get_secret_value(), {"interactive": True}
  62         )
  63     except AccessDenied:
! 64         return endpoint._invalid_credentials_response()
! 65     return endpoint._authenticated_response(
  66         user_record, request.cookies.get(DEVICE_COOKIE)
  67     )
  68 
  69 
! 70 @pms_api_router.post(
  71     "/login/mfa",
  72     status_code=204,
  73     responses={401: UNAUTHORIZED_RESPONSE, 204: {"model": None}},
  74     tags=["login"],

---

Lines 72-80

  72     status_code=204,
  73     responses={401: UNAUTHORIZED_RESPONSE, 204: {"model": None}},
  74     tags=["login"],
  75 )
! 76 async def login_mfa(verification: PmsMfaInput, request: Request, env: PublicEnv):
  77     """
  78     Finishes a login that was answered with `/errors/mfa-required`.
  79 
  80     A wrong code can be tried again. Once the answer is

---

Lines 79-87

  79 
  80     A wrong code can be tried again. Once the answer is
  81     `/errors/mfa-too-many-attempts` the whole login has to start over.
  82     """
! 83     return env["pms.fastapi.login.endpoint"]._verification_response(
  84         request.cookies.get(MFA_COOKIE),
  85         verification.code,
  86         verification.remember,
  87         request.headers.get("user-agent"),

---

Lines 108-117

  108 class PmsFastapiLoginEndpoint(models.AbstractModel):
  109     _name = "pms.fastapi.login.endpoint"
  110     _description = "login endpoint helper"
  111 
! 112     def _invalid_credentials_response(self):
! 113         return self._problem_response(
  114             "/errors/invalid-credentials",
  115             _("Invalid credentials"),
  116             _("Wrong user or password."),
  117         )

---

Lines 115-124

  115             _("Invalid credentials"),
  116             _("Wrong user or password."),
  117         )
  118 
! 119     def _problem_response(self, type_, title, detail):
! 120         return JSONResponse(
  121             status_code=status.HTTP_401_UNAUTHORIZED,
  122             media_type="application/problem+json",
  123             content={
  124                 "type": type_,

---

Lines 127-154

  127                 "detail": detail,
  128             },
  129         )
  130 
! 131     def _authenticated_response(self, user_record, device_token):
  132         """Either a session, or the demand for a second factor."""
! 133         mfa_type = user_record._mfa_type()
! 134         if not mfa_type:
! 135             return self._get_login_response_with_cookies(user_record)
! 136         if device_token and self._is_remembered_device(user_record, device_token):
! 137             return self._get_login_response_with_cookies(user_record)
! 138         if mfa_type != SUPPORTED_MFA_TYPE:
  139             # Letting an account through because its second factor is one this
  140             # API cannot ask for would turn the protection into a hole.
! 141             return self._problem_response(
  142                 "/errors/mfa-required",
  143                 _("Second factor required"),
  144                 _("This account cannot be used from here."),
  145             )
! 146         return self._demand_verification_response(user_record)
  147 
! 148     def _demand_verification_response(self, user_record):
! 149         token = secrets.token_urlsafe(32)
! 150         self.env["pms.fastapi.mfa.challenge"].sudo().create(
  151             {
  152                 "user_id": user_record.id,
  153                 "token": token,
  154                 "expire": fields.Datetime.now() + timedelta(seconds=MFA_MAX_AGE),

---

Lines 153-197

  153                 "token": token,
  154                 "expire": fields.Datetime.now() + timedelta(seconds=MFA_MAX_AGE),
  155             }
  156         )
! 157         response = self._problem_response(
  158             "/errors/mfa-required",
  159             _("Second factor required"),
  160             _("Send the code shown by your authentication app."),
  161         )
! 162         self._set_cookie(response, MFA_COOKIE, token, MFA_MAX_AGE)
! 163         return response
  164 
! 165     def _verification_response(self, token, code, remember, user_agent=None):
! 166         challenge = self._open_challenge(token)
! 167         if not challenge:
! 168             return self._start_over_response()
! 169         user_record = challenge.user_id
! 170         try:
! 171             with user_record._assert_can_auth(user=user_record.id):
! 172                 user_record._totp_check(int(re.sub(r"\s", "", code)))
! 173         except (AccessDenied, ValueError):
  174             # Returned and not raised: the attempt has to survive the answer,
  175             # or a wrong code would cost the caller nothing.
! 176             challenge.attempts += 1
! 177             if challenge.attempts >= MFA_MAX_ATTEMPTS:
! 178                 challenge.unlink()
! 179                 return self._start_over_response()
! 180             return self._problem_response(
  181                 "/errors/mfa-invalid-code",
  182                 _("Invalid code"),
  183                 _("The code is not valid."),
  184             )
! 185         challenge.unlink()
! 186         response = self._get_login_response_with_cookies(user_record)
! 187         if remember:
! 188             self._remember_device(user_record, response, user_agent)
! 189         self._set_cookie(response, MFA_COOKIE, "", 0)
! 190         return response
  191 
! 192     def _start_over_response(self):
! 193         return self._problem_response(
  194             "/errors/mfa-too-many-attempts",
  195             _("Verification no longer possible"),
  196             _("Log in again."),
  197         )

---

Lines 195-212

  195             _("Verification no longer possible"),
  196             _("Log in again."),
  197         )
  198 
! 199     def _open_challenge(self, token):
! 200         Challenge = self.env["pms.fastapi.mfa.challenge"].sudo()
! 201         if not token:
! 202             return Challenge
! 203         return Challenge.search(
  204             [("token", "=", token), ("expire", ">", fields.Datetime.now())], limit=1
  205         )
  206 
! 207     def _is_remembered_device(self, user_record, token):
! 208         return (
  209             self.env["auth_totp.device"]
  210             .sudo()
  211             ._check_credentials_for_uid(
  212                 scope=DEVICE_SCOPE, key=token, uid=user_record.id

---

Lines 212-223

  212                 scope=DEVICE_SCOPE, key=token, uid=user_record.id
  213             )
  214         )
  215 
! 216     def _remember_device(self, user_record, response, user_agent):
  217         # Generated as the user: the key belongs to whoever is logging in, not
  218         # to the account running the API.
! 219         key = (
  220             self.env["auth_totp.device"]
  221             .with_user(user_record)
  222             .sudo()
  223             ._generate(DEVICE_SCOPE, self._device_name(user_agent))

---

Lines 221-239

  221             .with_user(user_record)
  222             .sudo()
  223             ._generate(DEVICE_SCOPE, self._device_name(user_agent))
  224         )
! 225         self._set_cookie(response, DEVICE_COOKIE, key, DEVICE_MAX_AGE)
  226 
! 227     def _device_name(self, user_agent):
  228         """What the user will read in the trusted devices list."""
! 229         return (user_agent or _("Unknown device"))[:128]
  230 
! 231     def _set_cookie(self, response, key, value, max_age):
! 232         validator = (
  233             self.env["auth.jwt.validator"].sudo()._get_validator_by_name("api_pms")
  234         )
! 235         response.set_cookie(
  236             key=key,
  237             value=value,
  238             max_age=max_age,
  239             path=validator.cookie_path or "/",

---

Lines 275-283

  275             secret=validator.secret_key,
  276             expire=validator.cookie_max_age,
  277         )
  278         response = Response(status_code=status.HTTP_204_NO_CONTENT)
! 279         self._set_cookie(
  280             response, validator.cookie_name, token, validator.cookie_max_age
  281         )
  282         return response

---

pms_fastapi/schemas/pms_login.py

Lines 7-14

   7     username: str
   8     password: SecretStr
   9 
  10 
! 11 class PmsMfaInput(PmsBaseModel):
! 12     code: str
! 13     remember: bool = False

---

roomdoo_fastapi/routers/login.py

Lines 1-5

! 1 import secrets
  2 
  3 from fastapi import HTTPException, Request
  4 
  5 from odoo import models

---

Lines 67-75

  67         response = super()._get_login_response_with_cookies(user_record)
  68         # Without something unique of its own the token is a pure function of
  69         # its expiry, so two logins within the same second produce the very
  70         # same string and the second one hits the uniqueness constraint.
! 71         payload = {"jti": secrets.token_urlsafe(16)}
  72         refresh_token = validator._encode(
  73             payload,
  74             expire=validator.refresh_cookie_max_age,
  75             secret=validator.refresh_token_secret,

---