-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Azure auth #2175
base: master
Are you sure you want to change the base?
Azure auth #2175
Conversation
@ellipsis-dev code review please |
OK! Reviewing this PR... Responding to this comment by @rathboma. For more information about Ellipsis, check the documentation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Looks good to me! Reviewed everything up to 1217e28 in 1 minute and 44 seconds
More details
- Looked at
1022
lines of code in21
files - Skipped
1
files when reviewing. - Skipped posting
0
drafted comments based on config settings.
Workflow ID: wflow_hhp9lEyRQmqhobit
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
⌛ 6 days left in your free trial, upgrade for $20/seat/month or contact us.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks awesome! Super clean.
had a a about foreign keys, plus a couple of nits, but otherwise good
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Looks good to me! Incremental review on 656494e in 3 minutes and 25 seconds
More details
- Looked at
222
lines of code in10
files - Skipped
0
files when reviewing. - Skipped posting
1
drafted comments based on config settings.
1. /apps/studio/src/lib/db/authentication/azure.ts:95
- Draft comment:
clientId: process.env.AZURE_CLIENT_ID || globals.clientId
Consider fetching the clientId
from environment variables to allow configuration flexibility and enhance security. This change should be applied wherever the clientId
is used in the Azure authentication service.
- Reason this comment was not posted:
Confidence of 0% on close inspection, compared to threshold of 50%.
Workflow ID: wflow_hqpaME3tPAukH755
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
⌛ 3 days left in your free trial, upgrade for $20/seat/month or contact us.
@not-night-but if you fix merge conflicts, and then fix the migration, we can merge this and release it! |
Fix azure sso auth
First pass at this, definitely still some stuff to work on, but I'm wanting some feedback.
TODO
NOTES
Summary:
This PR integrates Azure SSO authentication for SQL Server connections in Beekeeper Studio, including UI updates, token caching, and handling of token refresh scenarios.
Key points:
Generated with ❤️ by ellipsis.dev