e-mission / op-admin-dashboard

An admin/deployer dashboard for the NREL OpenPATH platform
0 stars 9 forks source link

QR codes don't show up on the token screen #47

Open shankari opened 1 year ago

shankari commented 1 year ago

The token screen shows the tokens but not the QR codes

Screenshot 2023-06-04 at 4 14 51 PM

Also, I recall @AlirezaRa94 saying that only urlsafe works; we should remove the other options since they don't work.

AlirezaRa94 commented 1 year ago

The token screen shows the tokens but not the QR codes

You need to create a directory named "qrcodes" under "assets" directory that stores QR codes. https://github.com/e-mission/op-admin-dashboard/blob/927817a95a9f7f9c4307f98cbfe2ef02584c047d/pages/tokens.py#L21

Also, I recall @AlirezaRa94 saying that only urlsafe works; we should remove the other options since they don't work.

The urlsafe and base64 options sometimes create tokens that contain "/". The only safe option is hex.

shankari commented 1 year ago

You need to create a directory named "qrcodes" under "assets" directory that stores QR codes.

Is there a reason that the python code can't just create this if it doesn't exist? The goal behind using Docker is that it should encapsulate all these setup steps. I should be able to set the environment variable and use docker-compose up -d and it should Just Work.

shankari commented 1 year ago

The urlsafe and base64 options sometimes create tokens that contain "/". The only safe option is hex.

I have not had issues with urlsafe, but hex is fine too. My point is that you should remove options that don't work.

AlirezaRa94 commented 1 year ago

You need to create a directory named "qrcodes" under "assets" directory that stores QR codes.

Is there a reason that the python code can't just create this if it doesn't exist? The goal behind using Docker is that it should encapsulate all these setup steps. I should be able to set the environment variable and use docker-compose up -d and it should Just Work.

Yes, you are right. The Dockerfile already creates this folder.

I think it doesn't display the QR codes because whenever we build the docker container, the "qrcodes" folder will be empty, and we will miss the created QR codes.

shankari commented 1 year ago

so then you can regenerate the QR codes when the tokens are regenerated? you should be able to generate and display QR codes inline and only save to file if we need to export...

shankari commented 1 year ago

Note that we also want to read the default prefix from a config and support subgroups: https://github.com/e-mission/op-admin-dashboard/pull/9#issuecomment-1411200094

Note also our previous comments about being able to delete tokens that don't look good any more. or maybe showing "new" tokens first or supporting by subgroup. https://github.com/e-mission/op-admin-dashboard/issues/21#issuecomment-1479849486

We don't support deletion in the CLI because more tokens doesn't hurt anything from a system perspective. But more tokens clutter up the UI. Is there a way to display more or to filter? Or maybe we should support deleting tokens as well, but with appropriate warnings.

AlirezaRa94 commented 1 year ago

Note that we also want to read the default prefix from a config and support subgroups: #9 (comment)

@shankari The default prefix already exists in the dynamic config (token_prefix). There is also an input field to add a program or study name to tokens. Could you please provide some more details about this comment?

shankari commented 7 months ago

@AlirezaRa94 I don't think we should have people specify the program or study name. It leaves open the potential for error, and if they mistype, then the tokens won't work. We should get the program or study name from the dynamic config, and put the default prefix into the static configuration (similar to cognito settings)

shankari commented 7 months ago

@achasmita the list of valid tokens is stored on the server. It is currently manipulated using the scripts in bin/auth on the server. As you can see, the code here is essentially a port of bin/auth to the admin dashboard. I think that creating the tokens actually works, but I am not sure that the QR code works, and deleting does not work.

shankari commented 7 months ago

You might also want to read the discussion here and plan out how to design the screen, potentially with help from @JGreenlee

achasmita commented 7 months ago

I can see the QR code is not visible but regenerating it at beginning, fixes the problem.

For deleting token:

Adding row_deltetable=True allow to delete each row:

Screen Shot 2024-01-29 at 9 28 32 PM