chore(friendshipper): move oidc auth + refresh logic into the backend #384
Conversation
rudoi
changed the title
There was a problem hiding this comment.
Wow, this is great! I think this shares SSO with the browser, whereas before they were independent, correct?
I'm definitely less clear on the JS interactions, and potential edge cases with the error handling around the backend. It looks like the token json is stored in localStorage, so across a restart the frontend will have the last-saved token, but how does that token get used by the backend? I think the answer is that the backend only accesses AWS with credentials provided by the frontend?
During testing (on Linux!) I got into a state where pages other than Home were reporting back only "500 Internal Error". This happens after using the Logout button in Prefs. After clicking logout it restarts the client, pops the browser window and that looks successful, but tauri fails to build any of the other pages.
Otherwise, everything seems solid. Other than the couple of questions, everything LGTM!
core/Cargo.toml
Outdated
| @@ -78,14 +78,15 @@ ureq = "2.6.2" | |||
| which = "4.4.0" | |||
| json-patch = "1.1.0" | |||
| ring = "0.17" | |||
| graphql_client = { version = "0.13.0", features = ["reqwest"] } | |||
| graphql_client = { git = "https://github.com/graphql-rust/graphql-client.git", rev = "9b91a7f7d4a21dbbeacf974bce63fe5e55620ca8", features = ["reqwest"] } | |||
There was a problem hiding this comment.
Worth calling out why this and openidconnect are pinned to git?
There was a problem hiding this comment.
for sure, i'll comment it (it's underlying dependency nonsense)
friendshipper/src-tauri/src/main.rs
Outdated
| let auth_handle = handle.clone(); | ||
| thread::spawn(move || { | ||
| while let Ok(tokens) = oidc_rx.recv() { | ||
| auth_handle.emit_all("oidc-tokens", &tokens).unwrap(); |
There was a problem hiding this comment.
Is this unwrap safe? Are there any non-catastrophic conditions where the emit_all can fail but the tauri app doesn't exit completely?
There was a problem hiding this comment.
I'll handle it instead. I've never run into an issue here but famous last words
| @@ -91,7 +95,16 @@ | |||
| } | |||
| }, | |||
| "security": { | |||
| "csp": null | |||
| "csp": { | |||
There was a problem hiding this comment.
Could you describe what this is doing?
There was a problem hiding this comment.
Actually this might not be necessary anymore. Let me do some testing. Originally I was popping the OIDC login into the friendshipper window directly, which requires some CSP modification. Lemme get back to you.
| @@ -415,7 +356,7 @@ | |||
| if ($appConfig.repoPath !== '') { | |||
| const [repoConfigResponse, repoStatusResponse, commitsResponse] = await Promise.all([ | |||
| getRepoConfig(), | |||
| getRepoStatus(SkipDllCheck.False, AllowOfflineCommunication.False), | |||
There was a problem hiding this comment.
Why do these parameters change?
There was a problem hiding this comment.
these are actually the function's defaults
rudoi
force-pushed
the
ar/okta-mac-2
branch
3 times, most recently
from
a6452eb to
b5a39f6
Compare
No description provided.