Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow the UI to store user preferences #926

Open
ctron opened this issue Oct 17, 2024 · 4 comments
Open

Allow the UI to store user preferences #926

ctron opened this issue Oct 17, 2024 · 4 comments
Assignees
Labels
frontend Frontend related code changes

Comments

@ctron
Copy link
Contributor

ctron commented Oct 17, 2024

Proposal

We create a simple CRUD API for user preferences. The endpoint looks like this:

(GET|PUT|POST|DELETE) /api/v1/preferences/{key}

Where key is any ID the UI picks. Values are unique by a combination of user + key. The user is derived from the OIDC bearer token provided during the request. The content is opaque to the backend. If a key is not present for a user, it returns a 404.

There is no way to list preferences. We could do that, but right now I don't see a use case.

Pros

  • Simple and efficient
  • The UI is in full control and can manage the state on its own. Changes in the UI don't require a change in the backend.
  • It aligns with our current architecture

Cons

  • There's no lifecycle management with the OIDC solution at this point. e.g. deleting a user in keycloak would leave data in our preference store.
@ctron ctron self-assigned this Oct 18, 2024
@ctron ctron added the frontend Frontend related code changes label Oct 18, 2024
@ctron
Copy link
Contributor Author

ctron commented Oct 18, 2024

@carlosthe19916 @chirino maybe you can take a look and check if that is what you'd need.

@carlosthe19916
Copy link
Member

Some context:

  • What we call "user preferences" in V1 is the 4 SBOMs the user wants to watch. The dashboard will render a donut chart of those 4 sboms every time the user enters to the dashboard
  • Those 4 SBOMs that the user want to watch are unique to each specific user. And only the user himself can update which are those 4 SBOMs he want to watch.

We can use this https://www.figma.com/design/go1K9B6nRryAqsyzZX8v9n/SPoG-useCases?node-id=720-30306&node-type=canvas&t=QTmiWaKbN9XlVqOr-0 as a reference

How it currently works in V1:

GET /api/v1/dashboard/userPreferences

  • If there is nothing in the database (the user has never saved his preferences) then the response is something like:
{
  "sbom1": null,
  "sbom2": null,
  "sbom3": null,
  "sbom4": null
}
  • If there is previous data in the DB then the response includes whatever sbom id the user saved before. E.g.
{
  "sbom1": null,
  "sbom2": "SBOM1_ID",
  "sbom3": null,
  "sbom4": "SBOM4_ID",
}

POST /api/v1/dashboard/userPreferences (we can change it to PUT as it looks more appropriate but that is what we have in V1)

The client sends exactly the same body he will get when using the GET Method. E.g.

{
  "sbom1": null,
  "sbom2": "SBOM1_ID",
  "sbom3": null,
  "sbom4": "SBOM4_ID",
}

And this value is saved in the DB

Notes:

  • The body of the endpoints are in the form of: "sbom1, sbom2, sbom3, sbom4". This is for being explicit which of the donut charts should be rendered and in which position.
  • When the client hits GET /api/v1/dashboard/userPreferences there is never a 404 but a body where the fields "sbom1,sbom2,sbom3,sbom4" are just null
  • The endpoint /api/v1/dashboard/userPreferences does not include any Path Parameter or Query Parameter, everything is inferred by the server through the token that is being sent.

Could we have something similar in this repository? We can rename the url endpoint and field names but the core principle could be the same.

I am not sure allowing the client to set key in /api/v1/preferences/{key} is a good idea. Not sure how key would be used. But from the description of the issue I assume the proposal is something like key=my_username+key_123456

  • It is an additional complexity that we might avoid perhaps?
  • There has to be a control for user1 not to get access to user2's key as only the user himself can read/write his own preferences

@ctron
Copy link
Contributor Author

ctron commented Oct 18, 2024

But from the description of the issue I assume the proposal is something like key=my_username+key_123456

The idea was to just use key_123456 and take the user ID from the bearer token:

The user is derived from the OIDC bearer token provided during the request.

Which basically means, that if you'd use the following endpoint: /api/v1/preferences/top5, you'd get the same. Only with the exception that in case that information is not set, you'd get a 404 instead.

Again on the pro side, the UI can choose new keys, without any changes on the backend. Translating a 404 into an empty (default) result, shouldn't be a problem for the UI.

  • It is an additional complexity that we might avoid perhaps?

What makes you assume additional complexity?

  • There has to be a control for user1 not to get access to user2's key as only the user himself can read/write his own preferences

That's ensured be the logic described above and in the original comment. Where do you see an issue?

@chirino
Copy link
Contributor

chirino commented Oct 19, 2024

I don't think the user id should be part of the path key. That should be extracted by the http handler from the auth token.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
frontend Frontend related code changes
Projects
Status: Backlog
Development

No branches or pull requests

3 participants