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

Feat/t 100 keycloak authentication #104

Open
wants to merge 30 commits into
base: main
Choose a base branch
from

Conversation

khalsz
Copy link
Collaborator

@khalsz khalsz commented Jan 15, 2025

This offers authentication/authorisation of fair-mast post api end based on user role (fair-mast-admin).
Below are demos.

  1. User Authentication.

image

  1. Unauthorised user post attempt (does not have fair-mast-admin role)

image

  1. Authorised user post attempt (has fair-mast-admin role)

image

@khalsz khalsz force-pushed the feat/T-100-keycloak-authentication branch 8 times, most recently from 1a5f96a to ce1d7de Compare January 30, 2025 15:34
@khalsz khalsz force-pushed the feat/T-100-keycloak-authentication branch 2 times, most recently from 412a50d to 4fed944 Compare February 5, 2025 12:19
@khalsz khalsz force-pushed the feat/T-100-keycloak-authentication branch from d76f52f to 4ec723a Compare February 7, 2025 16:10
@khalsz khalsz force-pushed the feat/T-100-keycloak-authentication branch 4 times, most recently from 78f0e73 to 7c7a4ee Compare February 17, 2025 14:33
@khalsz khalsz force-pushed the feat/T-100-keycloak-authentication branch from 7c7a4ee to c6fab43 Compare February 17, 2025 14:36
@khalsz khalsz force-pushed the feat/T-100-keycloak-authentication branch from 226d2dc to ec761d7 Compare February 18, 2025 09:39
@khalsz khalsz force-pushed the feat/T-100-keycloak-authentication branch from 133c8e8 to dbecc0d Compare February 18, 2025 10:14
@khalsz khalsz marked this pull request as ready for review February 18, 2025 10:44
df.to_sql("shots", engine, if_exists="append", index=False)
return shot_data
except Exception as e:
raise HTTPException(status_code=400, detail=f"Error:{str(e)}")
Copy link
Member

Choose a reason for hiding this comment

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

Why are you masking the real exception and returning an HTTPException? Could this not become hard to debug if the exception is actually coming from pandas?

df.to_sql("signals", engine, if_exists="append", index=False)
return signal_data
except Exception as e:
raise HTTPException(status_code=400, detail=f"Error:{str(e)}")
Copy link
Member

Choose a reason for hiding this comment

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

and here

df.to_sql("cpf_summary", engine, if_exists="append", index=False)
return cpf_data
except Exception as e:
raise (HTTPException(status_code=400, detail=f"Error:{str(e)}"))
Copy link
Member

Choose a reason for hiding this comment

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

and here

@NathanCummings
Copy link
Member

A few questions from me:

  1. Is it correct that those .db files in realm config are in the repo?
  2. Could you please clean up any commented-out code.
  3. Can you talk me through how you're handling the secrets now so I understand better? I'm not clear on what is dev and what is prod right now.

@khalsz
Copy link
Collaborator Author

khalsz commented Mar 12, 2025

A few questions from me:

  1. Is it correct that those .db files in realm config are in the repo?
  2. Could you please clean up any commented-out code.
  3. Can you talk me through how you're handling the secrets now so I understand better? I'm not clear on what is dev and what is prod right now.
  1. The .db files in the realm_config folder is a persistent file that contains config info about realm, client, user and more, used when docker is creating the keycloak instance (in GitHub ci/cd or any other env), so that a new keycloak instance with no realm, client, secret, user information is not created.
  2. Thank for point that out.

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