Skip to content

Add per-user API keys with global fallback - #46

Open
tinkermesomething wants to merge 3 commits into
Dodelidoo-Labs:mainfrom
tinkermesomething:feature/per-user-api-keys
Open

Add per-user API keys with global fallback#46
tinkermesomething wants to merge 3 commits into
Dodelidoo-Labs:mainfrom
tinkermesomething:feature/per-user-api-keys

Conversation

@tinkermesomething

Copy link
Copy Markdown

Users can now configure their own Last.fm, YouTube, and LLM API keys in their Profile page. When set, user keys override the global keys for that user's sessions. If not set, falls back to admin-configured global keys automatically.

  • Add API key columns to User model
  • Add migration for new columns
  • Add getter methods to DataHandler with user->global fallback
  • Update preview/prehear/ai_prompt to use per-user keys
  • Add API key form fields to profile page
  • Handle API key form submission in profile update

@smileBeda smileBeda left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code wise all OK, now testing locally and will merge if passed

@smileBeda

Copy link
Copy Markdown
Member

I see one issue which is not related to functionality or code, but safety:

  • previously, only the super-admins could add such keys and only those saw the keys.
  • now, every the machine owner (not the super-admin but the person who set the app up) can access the DB and see user's API Keys and misuse them.

However, there is only ONE true safe approach to this, which is, the user has to input a decryption key EVERY TIME the api key is used. Then, the api keys could be stored encrypted and NO ONE could read them.

This is cumbersome UI.

So I propose this:
Add a warning on the relevant sections for the users (not the super-admin of course) that IF they provide BYO Keys, the owner of the machine where this app is hosted, CAN read them, and that they do so at their own risk.
Something like:

Warning: Your API key will be stored on this server in order to use it. If you don’t trust the server owner or host, do not enter your key.

What do you think?

@tinkermesomething

Copy link
Copy Markdown
Author

I agree with your assessment and a warning for now is the easiest workaround. I'll sort it in the coming days :)
Down the line, we could potentially use the user's password to encrypt the keys in the DB and make it proper, no?

@smileBeda

Copy link
Copy Markdown
Member

Ah right, I did not think of that! Although I can reset that password as machine owner, it is as close as it gets to real security.
I guess we could make it super safe by NOT re-encrypting the keys on password reset, and instead ask them to re-add the key, thus avoiding some machine admin getting keys by simply resetting passwords.


There is another UI issue, when you add a BYO OpenAI api key and do NOT add the URL for the endpoint - but global overwrites it with something else (custom url), then it tries to use that URL
I suggest just putting again a simple single warning there, no need to do fancy checks in code. Something like
"If you BYO API key also make sure to add the correct URL. Note, local URLs will not work unless on the same machine as the app is hosted"

So I think after adding these 2 warnings (phrase them as you like, above are just a mix of AI and my own wording) we can release this. Then, probably after that add a new ticket for handling the security aspect.

@tinkermesomething
tinkermesomething force-pushed the feature/per-user-api-keys branch 2 times, most recently from c889592 to fe2bbdb Compare January 21, 2026 21:47
@tinkermesomething
tinkermesomething force-pushed the feature/per-user-api-keys branch from fe2bbdb to a52ffd2 Compare February 15, 2026 01:33
@tinkermesomething

Copy link
Copy Markdown
Author

@smileBeda Hiya :) sorry for the delay! I update the PR. all the best :)

@smileBeda

Copy link
Copy Markdown
Member

Hi @tinkermesomething - sorry the delay too, lol.
First off, I made one edit on the alembic migration because it was pointing to an older downgrade and upgrade from than we released. I already committed that to your PR directly. I think this might have happened because your side of the code was not synced with the newest from main?


I then tested the new BYO key features and they seem to work for example for YT, but not for LastFM, which still requires a global key (even if I set my own, it will say Administrator must configure Last.fm API keys in Settings.)

Looking at the code it seems

  • get_lastfm_api_key/secret are used in preview and prehear (data_handler.py:1434, data_handler.py:1471).
  • The "Last.fm for me" button state and request gate check self.last_fm_user_service instead (data_handler.py:791,
    data_handler.py:835).
  • self.last_fm_user_service is only built from global admin keys (data_handler.py:1824).

As such the lastFM button will always be disabled (unless using global keys if provided).


I did not physically test the other keys (Openai, Listebrainz user, and so on), but looking at the code this is what I see:

  1. Last.fm: broken for BYO-only in discovery flows.
  • Personal source readiness and error gating depend on self.last_fm_user_service, which is only built from global admin keys:
  • Core artist-card/discovery pipelines still use global Last.fm keys directly, not per-user fallback
  • Only preview/prehear use per-user Last.fm getters: data_handler.py:1434, data_handler.py:1471.
  1. YouTube: BYO works as intended.
  2. OpenAI: mostly correct, with one nuance.
  • If user sets only per-user model/headers/max-seeds but no per-user key/base, code reuses global recommender and ignores those per-user tuning fields. I guess that is fine, but would need to be communicated probably (or adjusted)
  1. ListenBrainz: no key override model exists, since no global key exists anyway, so that one should be fine

@smileBeda

Copy link
Copy Markdown
Member

PS I made a release which has yet another alembic migration so my edits to your PR will be wrong as well now.

Users can now configure their own Last.fm, YouTube, and LLM API keys
in their Profile page. When set, user keys override the global keys
for that user's sessions. If not set, falls back to admin-configured
global keys automatically.

- Add API key columns to User model
- Add migration for new columns
- Add getter methods to DataHandler with user->global fallback
- Update preview/prehear/ai_prompt to use per-user keys
- Add API key form fields to profile page
- Handle API key form submission in profile update
- Security warning: keys are stored on the server, use at own risk
- LLM URL warning: set correct base URL when using own key
- TODO comment for future key encryption via user password
- Fix get_openai_recommender_for_user to check global recommender
  before API key gate (was returning None when recommender was set
  but no raw key strings configured)
- Restore _personal_source_definitions to no-args signature to avoid
  breaking test monkey-patches; BYO Last.fm override applied inline
  in personal_recommendations after user resolution
- Fix _fetch_lastfm_top_tracks and prehear to use per-user Last.fm
  and YouTube keys via getter methods
- Fix personal discovery flow to support BYO-only Last.fm users with
  no global admin keys configured
- Rebase onto upstream/main; update migration down_revision to
  20260303_01
@tinkermesomething
tinkermesomething force-pushed the feature/per-user-api-keys branch from a672e5d to dc60b8d Compare March 10, 2026 22:20
@tinkermesomething

Copy link
Copy Markdown
Author

all fixed on my end :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants