github/penpot
1
0
Fork 0
mirror of https://github.com/penpot/penpot.git synced 2026-11-10 03:50:33 +00:00
Code Issues Packages Projects Releases Wiki Activity

✨ Add a session origin model to auth events (#12856)

Browse Source 
* 🐛 Return the full profile from the LDAP invitation branch

The invitation branch responded with only {:invitation-token token},
so the session start received a map without id or auth-backend. With
the new auth-origin vocabulary that map fails validation and the login
breaks in development and tests; in production the event would be
emitted without a channel.

Return the profile plus the token, like the regular branch, and cover
both branches with RPC-level tests.

Closes #12849

AI-assisted-by: deepseek-v4.1-flash

* 🐛 Mint an auth token in the OIDC callback with the invitation inside

The callback used the invitation JWE as the session token, so the
frontend landed directly on token verification and never ran the
regular session start: no signin event, no teams, no host hook.

Always mint an :auth token and carry the invitation in its claims.
Token verification returns the claims verbatim, so login-from-token
attaches the invitation to the profile and logged-in does the rest,
the same pattern the verify-email token already uses.

Closes #12849

AI-assisted-by: deepseek-v4.1-flash

* ✨ Keep the session origin in the telemetry shadow rows

The anonymisation filter drops every string unless it is on a short
allowlist. Add source and activation next to lang and auth-backend so
the shadow copy keeps them; the email and the full name are still
dropped and the email domain is still derived from the email.

The filter keeps its two bindings: the transducer discards everything
that is not a UUID, a boolean or a number, so the allowlist is applied
on the filtered map, never on the raw props.

Closes #12849

AI-assisted-by: deepseek-v4.1-flash

* ✨ Describe the signup outcome with activation, not with action

The three success branches of register-profile labelled a new account
as "login", "email-verification" or "accept-invitation", so the event
contradicted its own name and the value only lived in context, where
the analytics consumers never read it.

Add activation to the event props and drop action from those branches:
the name already says it is a signup, and a constant action would only
add noise. The retry branches keep their action in context because
there it distinguishes four different outcomes.

Closes #12849

AI-assisted-by: deepseek-v4.1-flash

* ✨ Describe the session origin in the signin event

The event carried the profile but never said why the session started:
the only field that tried was frontend metadata that never leaves the
browser, and several flows filled it with the wrong value.

Add a small closed vocabulary (source, auth-backend, activation,
from-invitation) and compose it from the profile when signin is
emitted. The assertion fails in development and tests and is elided in
the release build, so a bad value cannot break a real login.

Closes #12849

AI-assisted-by: deepseek-v4.1-flash

* ✨ Emit the session origin from every login entry point

The four emitters stamped a free-form string as metadata, so the
session event could not rely on it: two flows that were signups wrote
the same value as a login.

Route them through origin-meta: password and LDAP logins declare
source login, register declares source register with activation
immediate, and login-from-token derives both from the token issuer:
:auth for a login and :verify-email for a signup that needed the
email confirmation.

Closes #12849

AI-assisted-by: deepseek-v4.1-flash

* 🐛 Send the invitation signups through the regular session start

Registering from a team invitation link navigated by hand to token
verification, so the session start never ran: no signin event, no
teams, no host hook. The same navigation was copied in four places.

Teach login-from-register to carry an optional invitation token,
extract the profile decoration into with-register-origin, and call it
from the four sites instead of the hand-written navigation.

Closes #12849

AI-assisted-by: deepseek-v4.1-flash

* 🐛 Resolve the invitation without waiting for the teams

Registering from an invitation link now goes through the session
start, and that start only navigates after the teams request
completes. fetch-teams has no error handling, so a network failure
there would leave the invitation unaccepted and the user on the signup
screen with a session already minted.

Move the invitation navigation out of the teams path: it emits as soon
as the profile carries a token, and the teams path emits nothing for
an invited profile so it cannot move the user away from the token
verification screen.

Closes #12849

AI-assisted-by: deepseek-v4.1-flash

* 📚 Document the session origin vocabulary and the invitation flow

Add a focused memory with the four fields, where each value comes from,
the table of flows, the LDAP exception and the invitation path through
logged-in, and link it from the frontend routing.

Update the audit-log memory: source and activation are now on the
telemetry allowlist, activation lives in props and action only exists
on the retry rows.

Closes #12849

AI-assisted-by: deepseek-v4.1-flash

* 📎 Cover the invitation session start and fix the auth memories

The invitation flows and the token decoration in login-from-token were
only covered by helper tests, so removing the guard that keeps the
teams path from navigating, or the token attach, broke the behaviour
silently.

Run the events through ptk/watch with a teams-fetched event as input:
an invited profile navigates to the token verification screen and
never to the dashboard, and the signin keeps from-invitation. Add the
login-from-token cases with and without the token, and fix the memory
statements about action, the flow table and from-invitation.

Closes #12849

AI-assisted-by: deepseek-v4.1-flash
This commit is contained in:
Andrey Antukh 2026-11-09 10:11:54 +01:00 committed by GitHub
parent 4f02995a3c
commit 0ea6d166f0
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
17 changed files with 636 additions and 77 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
.serena/memories/audit-log.md
View File

@ -250,9 +250,9 @@ Result: crash reports only exist if the frontend collector runs and `push-audit-
## 11. Consumers III: telemetry (shadow rows)
Telemetry reuses the same table with **anonymized shadow rows** (`source LIKE 'telemetry:%'`): day-truncated times, `0.0.0.0` IPs, props reduced to uuid/boolean/number values plus a few allowed fields (`lang`, `auth-backend`, derived `email-domain` — never raw emails), and a minimal context allowlist. Full + shadow rows can exist together per event when both flags are on; that duplication is on purpose.
Telemetry reuses the same table with **anonymized shadow rows** (`source LIKE 'telemetry:%'`): day-truncated times, `0.0.0.0` IPs, props reduced to uuid/boolean/number values plus a few allowed fields (`source`, `lang`, `auth-backend`, `activation`, derived `email-domain` — never raw emails), and a minimal context allowlist. Full + shadow rows can exist together per event when both flags are on; that duplication is on purpose.
Filters (`filter-telemetry-props` / `filter-telemetry-context`): by default only uuid/boolean/number with a simple key; exceptions: login/register/update-profile and frontend `identify` keep `lang`/`auth-backend`/`email-domain`; `organization-sso-auth-failed` keeps `failure-reason` if it is a known one; `instance-start` passes whole; frontend `navigate` keeps `[:route :file-id :team-id :page-id]`. Free text drops from telemetry but **stays in the full row and in Nexus**.
Filters (`filter-telemetry-props` / `filter-telemetry-context`): by default only uuid/boolean/number with a simple key; exceptions: login/register/update-profile and frontend `identify` keep `source`/`auth-backend`/`activation`/`email-domain`; `organization-sso-auth-failed` keeps `failure-reason` if it is a known one; `instance-start` passes whole; frontend `navigate` keeps `[:route :file-id :team-id :page-id]`. Free text drops from telemetry but **stays in the full row and in Nexus**. In `register-profile`, `activation` is a prop (it is what reaches PostHog). `action` exists only in the `context` of the full `register-profile-retry` rows; `filter-telemetry-context` drops it from the anonymised copy, which keeps only `:version`, `:initiator`, `:client-version` and `:client-user-agent`.
Shipping (`backend/src/app/tasks/telemetry.clj`): the cron sends shadows to `:telemetry-uri` as JSON in 10k batches and **deletes** them on success; it purges leftovers older than 7 days. It also sends a legacy report (counters, teams, email domains...). On official hosts nothing is collected or sent (`telemetry-excluded?` covers `penpot.app`/`penpot.dev`).

101
.serena/memories/frontend/auth-events.md Normal file
View File

@ -0,0 +1,101 @@
# Auth Events and the Session Origin
`signin` is the frontend `identify` event emitted once per session from
`logged-in` (`app.main.data.auth`). It carries the profile plus a closed
vocabulary that says why the session started.
## Vocabulary
- `source`: `login` | `register`. Always present.
- `auth-backend`: `penpot` | `ldap` | `google` | `github` | `gitlab` | `oidc`.
Same key the profile already uses. Always present.
- `activation`: `immediate` | `email-verification`. Only on signups.
- `from-invitation`: boolean. Always present in `signin`
(`(some? (:invitation-token profile))`); `auth-origin` omits it when `nil`.
`auth-origin` (public, validated against `schema:auth-origin`) drops the nil
fields and checks with `(assert (sm/check ...))`: it throws in development and
tests, and `:elide-asserts true` removes it from the release build, so a bad
value can never break a real login. Emitters never call it directly.
## Where each value comes from
- `source` and `activation` travel as profile metadata (`::ev/source`,
`::ev/activation`), written by `origin-meta`. `login-from-token` derives both
from the token issuer with `iss->origin`: `:auth` → `login`; `:verify-email` →
`register` + `email-verification`. An unknown `:iss` produces no metadata, so
the assertion fires in development and the event goes without `:source` in
production.
- `auth-backend` and `from-invitation` are read from the profile.
- `profile-origin` composes the four and `logged-in` merges them into the event;
the emitters only declare the fact.
| Flow | `source` | `activation` | `from-invitation` |
|---|---|---|---|
| Password login | login | | false |
| Password login from an invitation link | login | | true |
| LDAP login | login | | false |
| Social login, existing active profile | login | | false |
| Signup, email verification disabled | register | immediate | false |
| Signup, email verification enabled | register | email-verification | false |
| Social signup, active profile | register | immediate | false |
| Social signup, inactive profile | register | email-verification | false |
| Signup from an invitation link, email verification disabled | register | immediate | true |
| Signup from an invitation link, email verification enabled | register | email-verification | true |
| Social login from an invitation link | login | | true |
| Social signup from an invitation link, email verification enabled | register | email-verification | true |
| LDAP login from an invitation link | login | | true |
LDAP is the explicit exception: the directory provisions the account on the
first login, but there is no user signup, so the flow is always `login` and
never carries `activation`. Do not "fix" it by inferring a signup.
## Invitations go through `logged-in`
- `login-from-register` accepts an optional `:invitation-token`;
`with-register-origin` decorates the profile and `logged-in` does the rest.
The four places that used to navigate by hand to `:auth-verify-token`
(`ui/auth/register.cljs` twice, `ui/viewer/login.cljs`, `ui/static.cljs`) emit
`login-from-register` with the token.
- The invitation navigates through `get-invitation-nav`, which does not wait for
`::dtm/teams-fetched`: `fetch-teams` has no error handling, so depending on it
would leave the invitation unaccepted when `get-teams` fails. For invited
profiles `get-redirect-events` emits `rx/empty`, so the teams path cannot move
the user away from the token verification screen.
- The OIDC callback with an invitation mints an `:auth` token that carries the
invitation JWE in its claims (`backend/src/app/auth/oidc.clj`), so the
frontend follows the regular path (`login-from-token` → `logged-in`). That
flow visits the token verification screen twice.
- `ui/viewer/login.cljs` and `ui/static.cljs` `:is-active` branches still bypass
`logged-in` (they hide the modal or reload the page); they are a separate
decision, not an oversight.
## Backend side
- The `register-profile` success branches
(`backend/src/app/rpc/commands/auth.clj`) put `activation` in the event props
and emit **no** `action`: the name already says it is a signup, and a constant
action would only add noise. The retry branches
(`register-profile-retry`) keep `action` in `context`
(`ignore-because-blocked`, `ignore`, `resend-email-verification`,
`ignore-because-complaints`), where it distinguishes outcomes.
- The LDAP invitation branch of `login-with-ldap` returns the full profile plus
`:invitation-token`; a partial map would break the `auth-origin` assertion.
- `register-profile` does not accept the invitation: it mints the session and
returns the token. The acceptance is `accept-team-invitation`, emitted from
`verify-token`.
- `filter-telemetry-props` keeps `source` and `activation` in the anonymised
row; `:action` is not on the allowlist.
## Analytics
- `register-profile` is a track event, so the props adapter forwards
`activation` with no extra work.
- `signin` is an identify event built from a closed list of profile keys. It
only reaches analytics if the identify branch of Nexus forwards `source`,
`activation` and `from_invitation` (`external/nexus`, another repository).
## `::ev/source` is no longer dead metadata
It is read by `profile-origin`. Do not remove it as unused, and keep the
vocabulary in sync with `schema:auth-origin` and the table above.

1
.serena/memories/frontend/core.md
View File

@ -46,6 +46,7 @@ Workspace behavior:
App shell and product flows:
- Routing, root app shell, websocket, and global errors: `mem:frontend/routing-app-shell-subtleties`.
- Authentication events, the session origin vocabulary, and the invitation flow: `mem:frontend/auth-events`.
- Dashboard and viewer flows: `mem:frontend/dashboard-viewer-subtleties`.
- Plugin JS API runtime inside the frontend app: `mem:frontend/plugin-api-to-cljs-binding`.

14
backend/src/app/auth/oidc.clj
View File

@ -1113,11 +1113,15 @@
:else
(let [sxf (session/create-fn cfg profile info)
token (or (:invitation-token info)
(tokens/generate cfg
{:iss :auth
:exp (ct/in-future "15m")
:profile-id (:id profile)}))
;; The session token is always an :auth one; the
;; invitation token, when present, travels inside its
;; claims so the frontend goes through the regular
;; token verification path and then `logged-in`.
token (tokens/generate cfg
{:iss :auth
:exp (ct/in-future "15m")
:profile-id (:id profile)
:invitation-token (:invitation-token info)})
;; If proceed, update profile on the database
profile (update-profile-with-info cfg profile info)

2
backend/src/app/loggers/audit.clj
View File

@ -353,6 +353,8 @@
props' (-> props'
(assoc :lang (:lang props))
(assoc :auth-backend (:auth-backend props))
(assoc :source (:source props))
(assoc :activation (:activation props))
(assoc :email-domain (email/get-domain (:email props)))
(d/without-nils))]
(assoc params :props props'))

13
backend/src/app/rpc/commands/auth.clj
View File

@ -543,8 +543,8 @@
;; return :invitation-token. The frontend redirects to
;; :auth-verify-token, which immediately accepts the
;; invitation.
;; - active + no/mismatched invitation → mint session
;; ("login" action). New profile, no further action.
;; - active + no/mismatched invitation → mint session.
;; New profile, no further action.
;; - not-active + matching invitation → send the
;; verify-email mail with the invitation token EMBEDDED
;; into the verify-email JWE. No session yet. When the
@ -557,7 +557,11 @@
created?
(let [accept-invitation? (and (some? invitation)
(= (:email profile)
(:member-email invitation)))]
(:member-email invitation)))
props (assoc props :activation
(if (:is-active profile)
"immediate"
"email-verification"))]
(cond
(and (:is-active profile) accept-invitation?)
(let [invitation (assoc invitation :member-id (:id profile))
@ -567,7 +571,6 @@
:invitation-token token}
(rph/with-transform (session/create-fn cfg profile claims))
(rph/with-meta {::audit/replace-props props
::audit/context {:action "accept-invitation"}
::audit/profile-id (:id profile)})))
(:is-active profile)
@ -575,7 +578,6 @@
(rph/with-transform (session/create-fn cfg profile claims))
(rph/with-meta
{::audit/replace-props props
::audit/context {:action "login"}
::audit/profile-id (:id profile)}))
:else
@ -589,7 +591,6 @@
:email (:email profile)}
(rph/with-meta
{::audit/replace-props props
::audit/context {:action "email-verification"}
::audit/profile-id (:id profile)})))))
;; When email verification is disabled and an inactive profile already

3
backend/src/app/rpc/commands/ldap.clj
View File

@ -89,7 +89,8 @@
:member-id (:id profile)
:member-email (:email profile))
token (tokens/generate cfg claims)]
(-> {:invitation-token token}
(-> (profile/strip-private-attrs profile)
(assoc :invitation-token token)
(rph/with-transform (session/create-fn cfg profile))
(rph/with-meta {::audit/props (:props profile)
::audit/profile-id (:id profile)})))

44
backend/test/backend_tests/auth_ldap_test.clj
View File

@ -7,9 +7,53 @@
(ns backend-tests.auth-ldap-test
(:require
[app.auth.ldap :as ldap-auth]
[app.common.time :as ct]
[app.rpc.commands.ldap :as ldap-rpc]
[app.tokens :as tokens]
[backend-tests.helpers :as th]
[clj-ldap.client :as ldap]
[clojure.test :as t]))
(t/use-fixtures :once th/state-init)
(t/use-fixtures :each th/database-reset)
;; --- login-with-ldap: the invitation branch returns the whole profile
(defn- mock-authenticate
[email fullname]
(fn [_cfg _params]
{:email email :fullname fullname :backend "ldap"}))
(t/deftest login-with-ldap-invitation-returns-full-profile
(t/testing "the invitation branch returns the profile, not only the token"
(let [email "ldap-invitation@example.com"
system (assoc th/*system* ::ldap-auth/provider {:host "localhost"})
token (tokens/generate system {:iss :team-invitation
:exp (ct/in-future {:hours 1})})]
(with-redefs [ldap-auth/authenticate (mock-authenticate email "Invited Person")]
(let [result (ldap-rpc/sm$login-with-ldap
system
{:email email
:password "secret"
:invitation-token token})]
(t/is (uuid? (:id result)))
(t/is (= email (:email result)))
(t/is (= "ldap" (:auth-backend result)))
(t/is (string? (:invitation-token result)))
(t/is (not= token (:invitation-token result))))))))
(t/deftest login-with-ldap-without-invitation-returns-profile
(t/testing "the regular branch keeps returning the profile without a token"
(let [email "ldap-plain@example.com"
system (assoc th/*system* ::ldap-auth/provider {:host "localhost"})]
(with-redefs [ldap-auth/authenticate (mock-authenticate email "Plain Person")]
(let [result (ldap-rpc/sm$login-with-ldap system {:email email
:password "secret"})]
(t/is (uuid? (:id result)))
(t/is (= email (:email result)))
(t/is (= "ldap" (:auth-backend result)))
(t/is (nil? (:invitation-token result))))))))
;; --- search-user: filter must be escaped (RED: currently not escaped)
(t/deftest search-user-escapes-email-in-filter

51
backend/test/backend_tests/auth_oidc_test.clj
View File

@ -417,6 +417,11 @@
[result]
(get-in result [::yres/headers "location"]))
(defn- location-token
"Extract the `token` query param from a redirect location."
[location]
(second (re-find #"[?&]token=([^&]+)" location)))
(t/deftest callback-param-error-redirects-to-login
(let [cfg (dissoc base-cfg :app.email/blacklist :app.email/whitelist)
request {:params {:error "access_denied"}}]
@ -506,6 +511,52 @@
(t/is (.contains loc "http://localhost:3449?screen=auth-verify-token&"))
(t/is (.contains loc "token=")))))))
(t/deftest callback-with-invitation-mints-auth-token-with-invitation-claim
(let [cfg (dissoc base-cfg :app.email/blacklist :app.email/whitelist)
invitation (tokens/generate cfg {:iss :team-invitation
:exp (ct/in-future {:hours 1})
:member-id test-profile-id
:member-email "u@e.com"})
state (make-state-token cfg {:invitation-token invitation})
request (default-request cfg :state state)]
(binding [cf/config {:public-uri "http://localhost:3449"}
cf/flags #{:registration}]
(with-redefs [app.auth.oidc/resolve-provider (constantly {:type "oidc" :id "oidc"})
app.auth.oidc/get-info (constantly {:email "u@e.com" :fullname "U"
:backend "oidc" :email-verified false
:invitation-token invitation
:props {}})
app.auth.oidc/get-profile (constantly test-profile)
app.auth.oidc/update-profile-with-info (fn [cfg profile info] profile)
app.loggers.audit/submit (constantly nil)]
(let [result (#'oidc/callback-handler cfg request)
loc (redirect-location result)
claims (tokens/decode cfg (location-token loc))]
(t/is (= :auth (:iss claims))
"the session token must be an :auth token, not the invitation one")
(t/is (= invitation (:invitation-token claims))
"the invitation token must travel inside the :auth claims")
(t/is (= test-profile-id (:profile-id claims))))))))
(t/deftest callback-without-invitation-mints-plain-auth-token
(let [cfg (dissoc base-cfg :app.email/blacklist :app.email/whitelist)
state (make-state-token cfg {})
request (default-request cfg :state state)]
(binding [cf/config {:public-uri "http://localhost:3449"}
cf/flags #{:registration}]
(with-redefs [app.auth.oidc/resolve-provider (constantly {:type "oidc" :id "oidc"})
app.auth.oidc/get-info (constantly {:email "u@e.com" :fullname "U"
:backend "oidc" :email-verified false
:props {}})
app.auth.oidc/get-profile (constantly test-profile)
app.auth.oidc/update-profile-with-info (fn [cfg profile info] profile)
app.loggers.audit/submit (constantly nil)]
(let [result (#'oidc/callback-handler cfg request)
loc (redirect-location result)
claims (tokens/decode cfg (location-token loc))]
(t/is (= :auth (:iss claims)))
(t/is (not (contains? claims :invitation-token))))))))
(t/deftest callback-gracefully-handles-unable-to-retrieve-user-info
(let [cfg (dissoc base-cfg :app.email/blacklist :app.email/whitelist)
state (make-state-token cfg {})

83
backend/test/backend_tests/loggers_audit_test.clj Normal file
View File

@ -0,0 +1,83 @@
;; This Source Code Form is subject to the terms of the Mozilla Public
;; License, v. 2.0. If a copy of the MPL was not distributed with this
;; file, You can obtain one at http://mozilla.org/MPL/2.0/.
;;
;; Copyright (c) KALEIDOS SUBSIDIARY SL
(ns backend-tests.loggers-audit-test
"Tests for the anonymisation filter applied to the telemetry shadow rows."
(:require
[app.common.uuid :as uuid]
[app.loggers.audit :as audit]
[clojure.test :as t]))
(def ^:private profile-id (uuid/next))
(defn- shadow-props
[event]
(:props (audit/filter-telemetry-props event)))
(t/deftest identify-keeps-the-session-origin-fields
(let [props (shadow-props {:source "frontend"
:type "identify"
:name "signin"
:props {:id profile-id
:auth-backend "penpot"
:source "register"
:activation "immediate"}})]
(t/is (= "register" (:source props)))
(t/is (= "immediate" (:activation props)))
(t/is (= "penpot" (:auth-backend props)))
(t/is (= profile-id (:id props)))))
(t/deftest identify-drops-personal-data-and-derives-the-domain
(let [props (shadow-props {:source "frontend"
:type "identify"
:name "signin"
:props {:id profile-id
:email "someone@example.com"
:fullname "Some One"}})]
(t/is (not (contains? props :email)))
(t/is (not (contains? props :fullname)))
(t/is (= "example.com" (:email-domain props)))))
(t/deftest backend-auth-event-keeps-activation-and-drops-action
(let [props (shadow-props {:source "backend"
:type "action"
:name "register-profile"
:props {:id profile-id
:activation "email-verification"
:action "register"}})]
(t/is (= "email-verification" (:activation props)))
(t/is (not (contains? props :action)))))
(t/deftest absent-origin-fields-are-not-emitted-as-nil
(let [props (shadow-props {:source "frontend"
:type "identify"
:name "signin"
:props {:id profile-id}})]
(t/is (not (contains? props :source)))
(t/is (not (contains? props :activation)))
(t/is (not (contains? props :auth-backend)))
(t/is (not (contains? props :email-domain)))))
(t/deftest strings-outside-the-allowlist-are-dropped
(let [props (shadow-props {:source "frontend"
:type "identify"
:name "signin"
:props {:id profile-id
:fullname "Some One"
:anything "else"}})]
(t/is (not (contains? props :fullname)))
(t/is (not (contains? props :anything)))))
(t/deftest events-outside-the-auth-branch-keep-the-default-filter
(let [props (shadow-props {:source "backend"
:type "action"
:name "create-file"
:props {:id profile-id
:team-id profile-id
:activation "immediate"}})]
(t/is (= profile-id (:id props)))
(t/is (= profile-id (:team-id props)))
(t/is (not (contains? props :activation)))))

44
backend/test/backend_tests/rpc_profile_test.clj
View File

@ -578,9 +578,15 @@
;; DO REGISTRATION
(let [data {::th/type :register-profile
:token @current-token}
out (th/command! data)]
out (th/command! data)
mdata (-> out :result meta)]
(t/is (nil? (:error out)))
(t/is (= 1 (:call-count @mock))))
(t/is (= 1 (:call-count @mock)))
;; The pending account describes itself with :activation, and
;; the event no longer carries an :action.
(t/is (= "email-verification"
(get-in mdata [:app.loggers.audit/replace-props :activation])))
(t/is (not (contains? (get-in mdata [:app.loggers.audit/context] {}) :action))))
(th/reset-mock! mock)
@ -633,7 +639,13 @@
;; No verification email sent
(t/is (= 0 (:call-count @mock)))
;; Session is minted
(t/is (seq (:app.rpc/response-transform-fns mdata))))
(t/is (seq (:app.rpc/response-transform-fns mdata)))
;; The immediately usable account describes itself with
;; :activation, and the event no longer carries an :action.
(t/is (= "immediate"
(get-in mdata [:app.loggers.audit/replace-props :activation])))
(t/is (false? (get-in mdata [:app.loggers.audit/replace-props :from-invitation])))
(t/is (not (contains? (get-in mdata [:app.loggers.audit/context] {}) :action))))
;; Force the profile back to inactive to simulate the case where it was
;; created before disable-email-verification was set
@ -911,11 +923,15 @@
(t/is (map? reg-result))
;; Active branch: invitation-token is echoed back and a session
;; is minted via `session/create-fn`.
;; is minted via `session/create-fn`. The event describes the
;; outcome with :activation and :from-invitation, not with an
;; :action.
(t/is (string? (:invitation-token reg-result)))
(t/is (seq (:app.rpc/response-transform-fns mdata)))
(t/is (= "accept-invitation"
(get-in mdata [:app.loggers.audit/context :action])))))))
(t/is (= "immediate"
(get-in mdata [:app.loggers.audit/replace-props :activation])))
(t/is (true? (get-in mdata [:app.loggers.audit/replace-props :from-invitation])))
(t/is (not (contains? (get-in mdata [:app.loggers.audit/context] {}) :action)))))))
(t/deftest prepare-and-register-with-invitation-and-enabled-registration-2
(let [itoken (tokens/generate th/*system*
@ -1241,13 +1257,15 @@
(t/is (not (contains? result :invitation-token))
"register-profile must not return an invitation-token bound to an existing victim profile"))
;; INVARIANT 3: the server must NOT have taken the
;; "accept-invitation" branch (which is the one that mints a
;; session). For an existing victim profile, the operation
;; should fall through to the harmless "repeated registry" path.
(t/is (not= "accept-invitation"
(get-in mdata [:app.loggers.audit/context :action]))
"register-profile must not run the accept-invitation branch for an existing victim profile")
;; INVARIANT 3: the operation must fall through to the harmless
;; "repeated registry" path, not to any branch that mints a
;; session. The retry path identifies itself with its own event
;; name and does not report the account as a completed signup.
(t/is (= "register-profile-retry"
(:app.loggers.audit/name mdata))
"register-profile must fall through to the retry path for an existing victim profile")
(t/is (nil? (get-in mdata [:app.loggers.audit/replace-props :activation]))
"an existing profile must not be reported as a completed signup")
;; The victim must remain inactive: nothing in this anonymous
;; flow should have flipped `is-active` to true.
(let [reloaded (th/db-get :profile {:id (:id victim)})]

127
frontend/src/app/main/data/auth.cljs
View File

@ -31,16 +31,70 @@
[{:keys [id]}]
(and (uuid? id) (not= id uuid/zero)))
;; --- AUTH ORIGIN
(def schema:auth-origin
[:map
[:source [:enum "login" "register"]]
[:auth-backend [:enum "penpot" "ldap" "google" "github" "gitlab" "oidc"]]
[:activation {:optional true} [:enum "immediate" "email-verification"]]
[:from-invitation {:optional true} :boolean]])
(defn auth-origin
"Canonical description of how an auth flow happened. The fields that
do not apply are omitted: a login carries no `:activation`."
[{:keys [source auth-backend activation from-invitation]}]
(let [origin (d/without-nils
{:source source
:auth-backend auth-backend
:activation activation
:from-invitation from-invitation})]
(assert (sm/check schema:auth-origin origin) "invalid auth-origin")
origin))
(defn- profile-origin
[profile]
(let [m (meta profile)]
(auth-origin
{:source (::ev/source m)
:activation (::ev/activation m)
:auth-backend (:auth-backend profile)
:from-invitation (some? (:invitation-token profile))})))
(def ^:private iss->origin
{:auth {:source "login"}
:verify-email {:source "register" :activation "email-verification"}})
(defn- origin-meta
[{:keys [source activation]}]
(d/without-nils {::ev/source source
::ev/activation activation}))
;; --- EVENT: login
(defn- get-invitation-nav
"Observable that emits the navigation to the token verification screen
when the profile carries an invitation token, and nothing otherwise.
It does not depend on the teams: `fetch-teams` has no error handling,
so waiting for it would leave the invitation unaccepted when that call
fails."
[profile]
(->> (rx/of (:invitation-token profile))
(rx/filter some?)
(rx/map #(rt/nav :auth-verify-token {:token %}))))
(defn- logged-in
"This is the main event that is executed once we have logged in
profile. The profile can proceed from standard login or from
accepting invitation, or third party auth signup or singin."
[profile]
(letfn [(get-redirect-events [teams]
(if-let [token (:invitation-token profile)]
(rx/of (rt/nav :auth-verify-token {:token token}))
(if (some? (:invitation-token profile))
;; The invitation navigates on its own, without waiting for
;; the teams; this branch must not move the user away from
;; the token verification screen.
(rx/empty)
(if-let [redirect-href (:login-redirect storage/session)]
(binding [storage/*sync* true]
(swap! storage/session dissoc :login-redirect)
@ -69,17 +123,22 @@
(ws/initialize)
(dtm/fetch-teams))
;; The invitation resolves on its own, without waiting for the
;; teams.
(->> (get-invitation-nav profile)
(rx/observe-on :async))
;; We schedule this event to be executed a bit later,
;; when the profile is already set
(->> (rx/of (ev/event {::ev/name "signin"
::ev/type "identify"
:id (:id profile)
:email (:email profile)
:auth-backend (:auth-backend profile)
:fullname (:fullname profile)
:is-muted (:is-muted profile)
:default-team-id (:default-team-id profile)
:default-project-id (:default-project-id profile)}))
(->> (rx/of (ev/event (merge {::ev/name "signin"
::ev/type "identify"
:id (:id profile)
:email (:email profile)
:fullname (:fullname profile)
:is-muted (:is-muted profile)
:default-team-id (:default-team-id profile)
:default-project-id (:default-project-id profile)}
(profile-origin profile))))
(rx/observe-on :async))
@ -135,7 +194,7 @@
(rx/filter is-authenticated?)
(rx/map (fn [profile]
(with-meta (merge data profile)
{::ev/source "login"})))
(origin-meta {:source "login"}))))
(rx/tap on-success)
(rx/map logged-in)
(rx/observe-on :async)))))
@ -161,7 +220,7 @@
(rx/tap on-success)
(rx/map (fn [profile]
(-> profile
(with-meta {::ev/source "login-with-ldap"})
(with-meta (origin-meta {:source "login"}))
(logged-in))))
(rx/catch on-error))))))
@ -193,7 +252,7 @@
(defn login-from-token
"Used mainly as flow continuation after token validation."
[{:keys [profile invitation-token] :as tdata}]
[{:keys [profile invitation-token iss] :as tdata}]
(ptk/reify ::login-from-token
ptk/WatchEvent
(watch [_ _ _]
@ -204,29 +263,35 @@
(assoc :invitation-token invitation-token)
:always
(with-meta {::ev/source "login-with-token"}))]
(with-meta (origin-meta (iss->origin iss))))]
(logged-in profile))))))))
(defn- with-register-origin
[profile invitation-token]
(-> profile
(cond-> (some? invitation-token)
(assoc :invitation-token invitation-token))
(with-meta (origin-meta {:source "register" :activation "immediate"}))))
(defn login-from-register
"Event used mainly for mark current session as logged-in in after the
user successfully registered using third party auth provider (in this
case we dont need to verify the email)."
[]
(ptk/reify ::login-from-register
ptk/WatchEvent
(watch [_ _ stream]
(rx/merge
(rx/of (dp/fetch-profile))
(->> stream
(rx/filter dp/profile-fetched?)
(rx/take 1)
(rx/map deref)
(rx/filter is-authenticated?)
(rx/map (fn [profile]
(with-meta profile
{::ev/source "register"})))
(rx/map logged-in)
(rx/observe-on :async))))))
([] (login-from-register nil))
([{:keys [invitation-token]}]
(ptk/reify ::login-from-register
ptk/WatchEvent
(watch [_ _ stream]
(rx/merge
(rx/of (dp/fetch-profile))
(->> stream
(rx/filter dp/profile-fetched?)
(rx/take 1)
(rx/map deref)
(rx/filter is-authenticated?)
(rx/map #(with-register-origin % invitation-token))
(rx/map logged-in)
(rx/observe-on :async)))))))
;; --- EVENT: logout

30
frontend/src/app/main/ui/auth/register.cljs
View File

@ -133,20 +133,20 @@
on-success
(mf/use-fn
(mf/deps on-success-callback)
(fn [params]
(fn [response]
(if (fn? on-success-callback)
(on-success-callback params)
(on-success-callback response)
(cond
(some? (:invitation-token params))
(let [token (:invitation-token params)]
(st/emit! (rt/nav :auth-verify-token {:token token})))
(some? (:invitation-token response))
(st/emit! (da/login-from-register
{:invitation-token (:invitation-token response)}))
(:is-active params)
(:is-active response)
(st/emit! (da/login-from-register))
:else
(do
(swap! storage/user assoc ::email (:email params))
(swap! storage/user assoc ::email (:email response))
(st/emit! (rt/nav :auth-register-success)))))))
on-register-profile
@ -293,22 +293,22 @@
on-success
(mf/use-fn
(mf/deps on-success-callback)
(fn [params]
(cf/external-notify-register-success (:id params))
(fn [response]
(cf/external-notify-register-success (:id response))
(if (fn? on-success-callback)
(on-success-callback (:email params))
(on-success-callback (:email response))
(cond
(some? (:invitation-token params))
(let [token (:invitation-token params)]
(st/emit! (rt/nav :auth-verify-token {:token token})))
(some? (:invitation-token response))
(st/emit! (da/login-from-register
{:invitation-token (:invitation-token response)}))
(:is-active params)
(:is-active response)
(st/emit! (da/login-from-register))
:else
(do
(swap! storage/user assoc ::email (:email params))
(swap! storage/user assoc ::email (:email response))
(st/emit! (rt/nav :auth-register-success)))))))
on-error

5
frontend/src/app/main/ui/static.cljs
View File

@ -11,7 +11,7 @@
[app.common.data :as d]
[app.common.exceptions :as ex]
[app.common.uuid :as uuid]
[app.main.data.auth :refer [is-authenticated?]]
[app.main.data.auth :as da :refer [is-authenticated?]]
[app.main.data.common :as dcm]
[app.main.data.nitrate :as dnt]
[app.main.errors :as errors]
@ -127,7 +127,8 @@
(fn [data]
(cond
(:invitation-token data)
(st/emit! (rt/nav :auth-verify-token {:token (:invitation-token data)}))
(st/emit! (da/login-from-register
{:invitation-token (:invitation-token data)}))
(:is-active data)
(st/emit! (rt/reload true))

5
frontend/src/app/main/ui/viewer/login.cljs
View File

@ -9,8 +9,8 @@
(:require
[app.common.logging :as log]
[app.config :as cf]
[app.main.data.auth :as da]
[app.main.data.modal :as modal]
[app.main.router :as rt]
[app.main.store :as st]
[app.main.ui.auth.login :refer [login-dialog*]]
[app.main.ui.auth.recovery-request :refer [recovery-request-page*]]
@ -72,7 +72,8 @@
(fn [data]
(cond
(:invitation-token data)
(st/emit! (rt/nav :auth-verify-token {:token (:invitation-token data)}))
(st/emit! (da/login-from-register
{:invitation-token (:invitation-token data)}))
(:is-active data)
(st/emit! (modal/hide))

184
frontend/test/frontend_tests/data/auth_test.cljs Normal file
View File

@ -0,0 +1,184 @@
;; This Source Code Form is subject to the terms of the Mozilla Public
;; License, v. 2.0. If a copy of the MPL was not distributed with this
;; file, You can obtain one at http://mozilla.org/MPL/2.0/.
;;
;; Copyright (c) KALEIDOS SUBSIDIARY SL
(ns frontend-tests.data.auth-test
(:require
[app.common.uuid :as uuid]
[app.main.data.auth :as da]
[app.main.data.event :as ev]
[app.main.data.team :as dtm]
[beicon.v2.core :as rx]
[cljs.test :as t :include-macros true]
[frontend-tests.helpers.async :as async]
[potok.v2.core :as ptk]))
;; --- Event runtime helpers
(defn- ^:async watch-events
"Runs the `watch` method of `event` with an empty state and the given
input `stream`, and returns the events it emits."
[event stream]
(let [events (atom [])]
(await (async/observe (ptk/watch event {} stream)
:on-next #(swap! events conj %)))
@events))
(defn- find-data-event
"First `ptk/data-event` in `events` whose `::ev/name` is `name`."
[events name]
(some (fn [event]
(when (and (= :app.main.data.event/event (ptk/type event))
(= name (::ev/name (deref event))))
event))
events))
(defn- navigate-events
[events]
(filter #(= :app.main.router/navigate (ptk/type %)) events))
(t/deftest auth-origin-keeps-only-the-fields-that-apply
(t/is (= {:source "login" :auth-backend "penpot"}
(da/auth-origin {:source "login" :auth-backend "penpot"})))
(t/is (= {:source "register" :auth-backend "google"
:activation "email-verification"}
(da/auth-origin {:source "register" :auth-backend "google"
:activation "email-verification"})))
(t/is (= {:source "register" :auth-backend "penpot"
:from-invitation true}
(da/auth-origin {:source "register" :auth-backend "penpot"
:from-invitation true}))))
(t/deftest auth-origin-rejects-values-outside-the-vocabulary
(t/is (thrown? js/Error
(da/auth-origin {:source "login-with-token"
:auth-backend "penpot"})))
(t/is (thrown? js/Error
(da/auth-origin {:source "login"
:auth-backend "foo"}))))
(t/deftest origin-meta-omits-the-fields-that-do-not-apply
(let [origin-meta @#'da/origin-meta]
(t/is (= {::ev/source "login"}
(origin-meta {:source "login"})))
(t/is (= {::ev/source "register" ::ev/activation "immediate"}
(origin-meta {:source "register" :activation "immediate"})))))
(t/deftest iss-maps-to-an-origin
(let [iss->origin @#'da/iss->origin]
(t/is (= {:source "login"} (iss->origin :auth)))
(t/is (= {:source "register" :activation "email-verification"}
(iss->origin :verify-email)))
(t/is (nil? (iss->origin :unknown)))))
(t/deftest profile-origin-composes-the-event-props
(let [profile-origin @#'da/profile-origin]
(t/is (= {:source "login" :auth-backend "google"
:from-invitation false}
(profile-origin
(with-meta {:auth-backend "google"} {::ev/source "login"}))))
(t/is (= {:source "login" :auth-backend "google"
:from-invitation true}
(profile-origin
(with-meta {:auth-backend "google" :invitation-token "tok"}
{::ev/source "login"}))))
(t/is (= {:source "register" :auth-backend "penpot"
:activation "email-verification"
:from-invitation false}
(profile-origin
(with-meta {:auth-backend "penpot"}
{::ev/source "register" ::ev/activation "email-verification"}))))))
(t/deftest with-register-origin-attaches-the-invitation-token
(let [with-register-origin @#'da/with-register-origin
profile (with-register-origin {:id "x"} "tok")]
(t/is (= {:id "x" :invitation-token "tok"} profile))
(t/is (= {::ev/source "register" ::ev/activation "immediate"}
(meta profile)))))
(t/deftest with-register-origin-keeps-the-profile-without-a-token
(let [with-register-origin @#'da/with-register-origin
profile (with-register-origin {:id "x"} nil)]
(t/is (= {:id "x"} profile))
(t/is (not (contains? profile :invitation-token)))
(t/is (= {::ev/source "register" ::ev/activation "immediate"}
(meta profile)))))
(t/deftest invitation-nav-only-emits-with-an-invitation-token
(let [get-invitation-nav @#'da/get-invitation-nav
collect (fn [profile]
(let [events (atom [])]
(rx/subs! #(swap! events conj %)
(get-invitation-nav profile))
@events))]
(t/is (= [] (collect {:auth-backend "penpot"})))
(let [events (collect {:auth-backend "penpot"
:invitation-token "tok"})
event (deref (first events))]
(t/is (= 1 (count events)))
(t/is (= :auth-verify-token (:id event)))
(t/is (= {:token "tok"} (:params event))))))
;; --- logged-in: the invitation does not wait for the teams
(t/deftest ^:async logged-in-with-invitation-navigates-to-token-verification
(let [logged-in @#'da/logged-in
origin-meta @#'da/origin-meta
profile (with-meta {:id (uuid/next)
:auth-backend "penpot"
:invitation-token "tok"}
(origin-meta {:source "login"}))
;; The teams arrive: the invitation must not be diverted to the
;; default team dashboard.
teams-fetched (ptk/data-event ::dtm/teams-fetched [])
events (await (watch-events (logged-in profile)
(rx/of teams-fetched)))
navs (navigate-events events)
nav (some-> (first navs) deref)
signin (some-> (find-data-event events "signin") deref)]
(t/is (= 1 (count navs)))
(t/is (= :auth-verify-token (:id nav)))
(t/is (= {:token "tok"} (:params nav)))
(t/is (not-any? #(= :app.main.data.common/go-to-dashboard-recent (ptk/type %))
events))
(t/is (true? (:from-invitation signin)))))
(t/deftest ^:async logged-in-without-invitation-goes-to-the-dashboard
(let [logged-in @#'da/logged-in
origin-meta @#'da/origin-meta
profile (with-meta {:id (uuid/next) :auth-backend "penpot"}
(origin-meta {:source "login"}))
teams-fetched (ptk/data-event ::dtm/teams-fetched [])
events (await (watch-events (logged-in profile)
(rx/of teams-fetched)))]
(t/is (empty? (navigate-events events)))
(t/is (some #(= :app.main.data.common/go-to-dashboard-recent (ptk/type %))
events))))
;; --- login-from-token: the invitation token survives the decoration
(t/deftest ^:async login-from-token-with-invitation-keeps-the-token
(let [[event] (await (watch-events
(da/login-from-token
{:profile {:id (uuid/next) :auth-backend "penpot"}
:iss :auth
:invitation-token "tok"})
(rx/empty)))
events (await (watch-events event (rx/empty)))
signin (some-> (find-data-event events "signin") deref)]
(t/is (= :app.main.data.auth/logged-in (ptk/type event)))
(t/is (= "login" (:source signin)))
(t/is (true? (:from-invitation signin)))))
(t/deftest ^:async login-from-token-without-invitation-has-no-invitation
(let [[event] (await (watch-events
(da/login-from-token
{:profile {:id (uuid/next) :auth-backend "penpot"}
:iss :auth})
(rx/empty)))
events (await (watch-events event (rx/empty)))
signin (some-> (find-data-event events "signin") deref)]
(t/is (false? (:from-invitation signin)))))

2
frontend/test/frontend_tests/runner.cljs
View File

@ -9,6 +9,7 @@
[frontend-tests.code-gen-style-test]
[frontend-tests.composable-tests.comp.sync-test]
[frontend-tests.copy-as-svg-test]
[frontend-tests.data.auth-test]
[frontend-tests.data.comments-filters-test]
[frontend-tests.data.dashboard-test]
[frontend-tests.data.event-test]
@ -162,6 +163,7 @@
'frontend-tests.code-gen-style-test
'frontend-tests.composable-tests.comp.sync-test
'frontend-tests.copy-as-svg-test
'frontend-tests.data.auth-test
'frontend-tests.data.comments-filters-test
'frontend-tests.data.dashboard-test
'frontend-tests.data.event-test