Update Authentication to use local storage and easy auth refresh #1117
Conversation
|
I think some snapshot tests need updating to reflect change to localStorage? |
| @@ -88,18 +89,24 @@ export const getRedirectUri = () => { | |||
| // Get an access token if a user logged in using app services authentication | |||
| // Returns null if the app doesn't support app services authentication | |||
| const getAppServicesToken = (): Promise<AppServicesToken | null> => { | |||
| return fetch(appServicesAuthTokenUrl).then(r => { | |||
| return fetch(appServicesAuthTokenRefreshUrl).then(r => { | |||
There was a problem hiding this comment.
So this is constantly refreshing? Is that the general recommendation? (Versus refreshing only based on expiration or some such?) Might be a naive question
There was a problem hiding this comment.
There doesn't seem to be a solid recommendation on how often to do this
There was a problem hiding this comment.
Okay, CC @jamesc in case he has any thoughts, but this seems fine to ship.
How do you do this? nvm, figured it out. it's not the snapshot tests it was authentication helper |
|
[like] James Casey reacted to your message:
________________________________
From: Pamela Fox ***@***.***>
Sent: Tuesday, January 9, 2024 12:15:47 AM
To: Azure-Samples/azure-search-openai-demo ***@***.***>
Cc: James Casey ***@***.***>; Mention ***@***.***>
Subject: Re: [Azure-Samples/azure-search-openai-demo] Update Authentication to use local storage and easy auth refresh (PR #1117)
@pamelafox commented on this pull request.
________________________________
In app/frontend/src/authConfig.ts<#1117 (comment)>:
@@ -88,18 +89,24 @@ export const getRedirectUri = () => {
// Get an access token if a user logged in using app services authentication
// Returns null if the app doesn't support app services authentication
const getAppServicesToken = (): Promise<AppServicesToken | null> => {
- return fetch(appServicesAuthTokenUrl).then(r => {
+ return fetch(appServicesAuthTokenRefreshUrl).then(r => {
Okay, CC @jamesc<https://github.com/jamesc> in case he has any thoughts, but this seems fine to ship.
—
Reply to this email directly, view it on GitHub<#1117 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AACDQCRPCOK2THUGA3TDPL3YNSD3HAVCNFSM6AAAAABBO3GBGOVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTQMJQGIZDGOJUHA>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
|
[like] James Casey reacted to your message:
…________________________________
From: Matt ***@***.***>
Sent: Tuesday, January 9, 2024 12:51:01 AM
To: Azure-Samples/azure-search-openai-demo ***@***.***>
Cc: James Casey ***@***.***>; Mention ***@***.***>
Subject: Re: [Azure-Samples/azure-search-openai-demo] Update Authentication to use local storage and easy auth refresh (PR #1117)
Merged #1117<#1117> into main.
—
Reply to this email directly, view it on GitHub<#1117 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AACDQCU2GXJ7QJWNJRMZBPLYNSH7LAVCNFSM6AAAAABBO3GBGOVHI2DSMVQWIX3LMV45UABCJFZXG5LFIV3GK3TUJZXXI2LGNFRWC5DJN5XDWMJRGQZDGOJYGMZTMMQ>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
|
Hello @mattgotteiner @pamelafox Upon refreshing the app page from the browser (Chrome), the developer tool reported error: Besides, Could you help to check further? Thanks! |
|
Thanks for reporting for. regarding .auth not found - this is expected if easy auth is not setup, we might need to add more checks for this to avoid refreshing. regarding the typescript compile error what version of node are you using? |
|
@mattgotteiner Thanks for the quick response! My node version is v20.11.0 |
…re-Samples#1117) * switch to localstorage; add refresh * run prettier * fix tests --------- Co-authored-by: Matt Gotteiner <magottei@microsoft.com>
Purpose
Does this introduce a breaking change?
Pull Request Type
What kind of change does this Pull Request introduce?
What to Check