1.8.0: multi-user accounts, profiles, LDAP removal, and mobile layout fixes - #724
Merged
Merged
Conversation
feat: Add private/public filter to videos view for authenticated user (develop branch)
Adds admin-managed user accounts, per-user permissions, media ownership, and shareable profile pages. Permissions - New permissions module holding the capability keys, presets (Viewer / Contributor / Curator), and the shared input validators. - Granted keys are stored as a JSON array on User, so an unknown or newly added key degrades to "not granted". The admin flag bypasses every check. - Applied across the ~45 mutating routes that were previously gated only by @login_required, plus per-object ownership checks on edit and delete. - view_private governs discovery (feeds, folders, tags, games, profiles). Direct access by id is deliberately unchanged: private stays link-only. Ownership - video.uploaded_by / image.uploaded_by, threaded from the upload endpoints through the background scan CLI. - edit_own / delete_own deliberately do not cover media with no uploader, so they cannot hand a freshly migrated library to every account. - Deleting a user never deletes their media; the uploader link is cleared. - Bulk "Set uploader" in File Manager for adopting existing content, and the upgrade attributes the existing library to the administrator account. Profiles - /u/<username> renders Open Graph metadata like /w/ and /i/, with matching nginx and vite proxy entries. - Profile pages reuse the existing feed components filtered by uploader. - Avatars are decoded and re-encoded to a 256x256 WebP, so only pixel data survives; a polyglot upload is stored inert. Files are named from the integer primary key, never from request content. - Clickable uploader byline on the video and image cards. Accounts - Settings > Users for administrators: create with a one-time setup link or an initial password, change access, reset credentials, clear 2FA, disable, or delete. Guardrails are enforced server-side. - Self-service password change in Settings > Security, plus a forced change for an account whose password an administrator chose. - The last enabled administrator cannot be demoted, disabled or deleted, and nobody can do those to their own account. - Disabling an account ends its access immediately, including remember-me. Fixes - Removed /api/signup. It was gated only by @login_required while User.admin defaulted to True, so any signed-in non-admin could create an administrator. - User.admin now defaults to False. `fireshare add-user` consequently creates non-admin accounts unless --admin is passed. - ADMIN_USERNAME / ADMIN_PASSWORD are re-applied on every boot to the account pinned by env_managed, rather than to an arbitrary "first local admin" that became ambiguous once a second administrator could exist. - The raw (uncut) video gate now resolves the video id and checks edit permission instead of admitting any authenticated session.
Clicking an image card on a profile did nothing. ImageCards has no modal of its own — its openImage is a no-op unless the parent supplies onImageOpen, and the viewer is owned by the page (ImageFeed does this). VideoCards, by contrast, renders its own VideoModal, which is why videos opened and images did not. Profile now owns EditImageModal the same way ImageFeed does, including next and previous navigation across the profile's own image list and propagating title, visibility, game and date edits back into the cards on close.
Header art - The banner now shows the art for whichever game the user uploads most, which is what the design called for and was never actually built — only the generated gradient shipped. Ties break on the lower game id so the choice is stable between requests. - A failed asset load falls back to the gradient underneath, since game art is only present when SteamGridDB assets were downloaded. - The banner credits the game and links through to its page. Banner uploads - Users can upload their own banner to override the game art, with the size hint (1920x620, roughly 3:1) and a note that anything else is centre-cropped. Removing it falls back to game art, then to the gradient. - Uploads go through the same hardened path as avatars, generalised into _store_profile_image: decoded and re-encoded to WebP so only pixel data survives, bounded before the body is read, and named from the integer primary key. Deleting an account now removes both files. Games tab - Profiles list the games the user has uploaded for, most-uploaded first, scoped to the media the viewer is allowed to see and annotated with their own contribution counts. - The asset cache-busting from /api/games is extracted into game_json_with_assets and shared, so both listings emit identical URLs and the browser cache carries across the two pages. - logo_url is populated whenever a game has a steamgriddb id, even when the asset was never fetched, so a load failure falls back to the game's name rather than leaving an unlabelled tile. Also adds the "last upload" relative timestamp beside the joined date, which the design showed and the implementation had dropped.
A profile has no top-bar controls of its own, so the app bar and the spacer reserving room for it were 64px of empty space above the banner. /profile now joins /files, /settings and /image in skipping both on desktop, which lets the banner start at the top of the content area. The bar is still rendered on mobile, where the drawer is off-canvas and its toggle is the only way to navigate — the same reason the other three keep it. The app bar and its spacer had the page list duplicated verbatim in two conditions, so they are now driven by one shared constant and flag instead.
A sweep of every mutating route as a view_private-only account found three reachable that should not have been: - GET/PUT /api/admin/config, which returns the whole server config including the SteamGridDB API key, and rewrites it on PUT. - GET /api/admin/warnings. - POST /api/admin/reset-database, which wipes the library. It answered 200. All three carried only @login_required and never had the inline admin check the rest of api/admin.py uses, so they were invisible when those checks were converted to decorators. Before per-user permissions existed every account was an administrator, which is why it did not surface until now. /api/admin/stream stays open to any signed-in account, with a comment saying why: the navbar transcoding and scan indicators subscribe to it for everyone, and it carries progress counts rather than configuration. Settings now requests the config and warnings only when the viewer is an administrator, and folder rules only with manage_games, instead of firing requests that 403 on every visit for an account that can only reach the Security tab. Folder rules are applied before the config early-return so a curator without administrator access still gets them.
The sort select was 150px and the All/Public/Private select 130px, both wider
than their content needs. Measured against the rendered labels: the longest
sort label ("Name A→Z") is 75px and the longest visibility label ("Private")
is 47px, and react-select adds 36px of indicator, 8px of value padding and 2px
of border. So the real minimums are ~121px and ~93px.
Sort is now 124px and visibility 96px at sm, which fits every label with no
truncation, verified by selecting the longest option in each and checking the
value does not overflow its container.
The two widths were duplicated across six views, so they now come from shared
constants. Applied to the folder, game, tag and folders views as well as videos
and images: it is the same control, and leaving four of them at the old width
would read as a bug rather than a choice. The folder select ("All Videos") is
untouched, since folder names are arbitrary and need the room.
The theme sets padding: 0 on MuiSwitch to get a compact 32x20 track, which also removes the padding that normally separates a switch from its label, so the text sat flush against the toggle. A Checkbox keeps its own padding and is inset from its visible box, which is why the permission checkboxes looked fine and only the switches did not. Adds an 8px label offset to the three switches in the user dialogs, via a named style with the reason attached so the next switch does not repeat it. The profile visibility switch already sits in a flex row with a gap and is unchanged.
LDAP authentication is removed. The directory login path, python-ldap and its system libraries, ldap_util, the ten LDAP_* settings and the `ldap` column all go. Removing it converts directory accounts to local accounts, which is a one-way change to somebody's database. An operator who still depends on LDAP should get to make that call, so create_app() refuses to start while any LDAP_* variable is set. Clearing them is the acknowledgement. The check has to run before the migration or the confirmation it asks for would come after the change it guards. FLASK_APP points at create_app, so the `flask db upgrade` step builds the app too and hits the check there; entrypoint.sh runs under `set -e`, so exiting non-zero stops the container with the database untouched. Directory accounts are converted rather than deleted. video.uploaded_by and image.uploaded_by reference user.id, so dropping the rows would orphan everything those accounts uploaded with no way to reattribute it. Every row keeps its id; uploads, profile and permissions survive. The account has no password, so an administrator sets one or sends an invite from Settings > Users — the two operations that used to answer 400 for a directory account and left the row unreachable. Admin status is left as it was at that account's last sign-in. It used to be re-derived from LDAP_ADMIN_GROUP on every login, so the stored value is a cache rather than a decision, but clearing it could remove the only administrator on an LDAP-only instance and this migration must not be what locks someone out of their own server. Also fixes the bootstrap admin adoption, which selected the lowest-id `admin=True, ldap=False` row. That column is gone after the migration, and on the upgrade run that applies it this code executes first, while directory rows are still present — so it would rename a directory admin to ADMIN_USERNAME and hand it the env password. It now requires a stored password, which is what made an admin local. On an LDAP-only instance no row qualifies and it falls through to creating a fresh local administrator, which is the account needed to invite everyone else back. BREAKING CHANGE: instances with an LDAP configuration will not start until the LDAP_* variables are removed. Pin to the previous release to keep using a directory server.
feat!: remove LDAP, refusing to start if it is still configured
Four issues raised on the PR. The File Manager nav entry was gated on manage_library, but every /api/admin/files* endpoint checks current_user.admin, so a non-admin holding that permission was shown a page whose every request answers 403. Verified: all six of those endpoints return 403 for a manage_library holder. The entry is now adminOnly, which the nav filter already supported, and FileManager refuses non-admins directly, since the route was reachable by any signed-in user regardless of what the nav showed. The ADMIN_USERNAME collision check compared usernames case-sensitively. The column has no NOCASE collation, so the unique constraint accepts a name differing only by case, but profile lookups resolve on lower(username) and account creation in api/users.py already rejects case-insensitive duplicates for exactly that reason. An operator setting ADMIN_USERNAME=shane against an existing "Shane" would have produced two rows that are indistinguishable in a URL, one of them unreachable. Now compared with lower() on both sides. profile_metadata.html claimed the username was passed through a JSON-encoded attribute via |tojson. It is not: it goes into a plain data attribute, which Jinja's autoescaping covers, and getAttribute hands back the decoded text without it ever reaching the JavaScript parser. The code was right and the comment was wrong, so the comment now describes the mechanism actually in use. Adding |tojson to an HTML attribute would mean JSON-in-HTML-escaped-in-an-attribute plus a parse on the way out, for no gain. The Settings fetch effect read isAdmin and can() with an empty dependency array. AuthWrapper withholds its children until the auth check resolves, so this was not the first-render bug it looks like, but AuthWrapper does recheck on a timer and on tab focus, and access changing under a mounted Settings would not refetch. The two access decisions are hoisted into booleans and used as the dependencies. Deliberately not `can` itself: it is rebuilt on every AuthWrapper render, so depending on the function would refetch continuously and overwrite unsaved edits in the config form.
Auditing the rest of the client for the mismatch raised on the file manager turned up the same defect one tab over. The Actions tab is revealed by manage_library, but Scan Games posts to /api/manual/scan-games, which is gated on manage_games. So a manage_library account saw a button that answered 403, and a manage_games account could not reach that scan at all, because the tab was hidden from them and the Actions tab is the only place it appears. Either permission now reveals the tab and the buttons inside it are gated individually: Scan Games on manage_games, the other five on manage_library. TAB_DEFS.perm accepts a list for this. Verified both directions: every button is shown only to an account the endpoint accepts, and each is refused for the permission that does not reveal it. Checked in the browser for manage_library alone (five buttons), manage_games alone (Actions tab now reachable, one button) and an administrator (all six).
Adds a divider above My Profile, splitting the browse pages from the account and admin group, and brings the rows in: 50px tall to 42, 18px labels to 15, 24px icons to 21, 8px gaps to 4. About 90px shorter overall. The divider is positioned by finding the first entry of that group which actually renders, rather than pinned to one href. The previous divider was keyed on File Manager, which is now admin-only, so it would have disappeared for everyone else. A signed-out visitor has none of the group and gets no divider at all instead of a leading one. The smaller icon also needed the minimized rail handled. The label collapses to zero width there but still flexes to fill, which pinned the icon to the label's indent; at 24px that happened to look centred and at 21px it did not. The label is now taken out of the flow when the drawer is closed and the icon is centred, so it holds at any icon size.
Adds a button beside each row's action menu that opens that account's profile in a new tab. It is a real anchor rather than window.open, so cmd-click, middle-click and the browser's own context menu behave normally. It targets /profile/<username> rather than the /u/<username> share link, which exists to serve Open Graph metadata and then redirects to the same place; nginx already falls back to index.html, so the direct route resolves on a fresh request. A hidden profile stays reachable by an administrator and this screen is admin-only, so the button never opens onto a 404.
feat: multi-user accounts with permissions, ownership, and profiles
The videos and images app bar packed the drawer toggle, three filter selects and the edit/search buttons into one row, which needs ~500px and so ran off the edge of a phone. Below sm the selects now collapse into a tune button that opens them in a popover, one labeled row each, with a dot on the button when a filter is off its default so the collapsed state does not hide active filtering. Desktop keeps the inline selects.
Edit profile, Add user and the two upload dialogs all carry a real form, which a centred paper leaves cramped on a phone. Below sm they now go fullScreen, via a useMobileFullScreenDialog hook that pairs the flag with an edge-to-edge paper style so the two cannot drift apart — the radius, border and shadow only read as artifacts once the paper meets the edges. The upload dialogs kept their desktop two-column split, which fullScreen turned into half-width fields under a band of dead space, so those columns stack below sm as well.
Done with npm version, which also resynced the lockfile: its top level had been left at 1.7.6 and the nested packages[""] entry at 1.7.0 by earlier hand-edited bumps.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Release 1.8.0. Rolls up 22 commits on
develop— 77 files, +6272/-836.Breaking change: LDAP is removed
create_app()now refuses to start while anyLDAP_*variable is set. Clearing them is the operator's acknowledgement, after which the migration converts directory accounts to local accounts — a one-way change to the database, which is why it is gated rather than silent.The check runs before the migration:
FLASK_APPpoints atcreate_app, soflask db upgradebuilds the app and hits the check there, andentrypoint.shruns underset -eso a still-configured deployment stops with the database untouched.Directory accounts are converted, not deleted —
video.uploaded_byandimage.uploaded_byreferenceuser.id, so dropping those rows would orphan their uploads with no way to reattribute them.Multi-user accounts, permissions, and ownership
Userso an unknown or newly added key degrades to "not granted". The admin flag bypasses every check.@login_required, plus per-object ownership checks on edit and delete.view_privategoverns discovery (feeds, folders, tags, games, profiles). Direct access by id is deliberately unchanged: private stays link-only.edit_own/delete_owndeliberately exclude media with no uploader, so a freshly migrated library isn't handed to every account. Deleting a user never deletes their media — the uploader link is cleared.Profiles
Shareable profile pages with header art, banner uploads, a games tab, and a link through from the Users settings screen.
Mobile layout fixes
smthe selects now collapse into a single button that opens them in a popover, with a dot when a filter is off its default so the collapsed state doesn't hide active filtering.smvia auseMobileFullScreenDialoghook that pairs the flag with an edge-to-edge paper style, so the two can't drift apart. The upload dialogs kept a desktop two-column split that fullscreen turned into half-width fields under a band of dead space, so those columns stack on mobile too.Migrations
Three, applied in order:
p1k2l3m4n5o6— users, permissions, profilesq2l3m4n5o6p7— profile bannerr3m4n5o6p7q8— remove LDAPReviewer notes
LDAP_*gets a stopped container rather than a converted database, which is intended but worth a second opinion on the messaging.