Support Sign in with Apple #3521
Conversation
Deploying matrix-authentication-service-docs with
|
| Latest commit: |
04e6960
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://1adae9d0.matrix-authentication-service-docs.pages.dev |
| Branch Preview URL: | https://quenting-siwa.matrix-authentication-service-docs.pages.dev |
sandhose
force-pushed
the
quenting/siwa
branch
from
313a9a2 to
c7d16c6
Compare
sandhose
force-pushed
the
quenting/siwa
branch
from
a24be69 to
f36e5ac
Compare
And make them available in the templates. This is useful to get the user display name for Sign-in with Apple
sandhose
force-pushed
the
quenting/siwa
branch
from
f36e5ac to
82e76f0
Compare
This is in preparation for also fetching the claims from the userinfo endpoint
There was a problem hiding this comment.
this all seems reasonable overall, albeit a lot of code for supporting one vendor (:/ :apple:...)!
Only thing giving me pause is losing the SameSite=Lax constraint on the cookies. When I was doing research into modern-day CSRF about a year ago, SameSite=Lax is probably the most important modern development for CSRF protection that prevents some types of attacks (called Session Fixation attacks or similar).
Losing that protection would be a shame, and I'd prefer to ask the security team before we did.
But I suspect we can find a workaround, which may or may not be palatable.
I haven't tried this myself, but it sounds like a client-side redirect (NOT server-side AFAICT) can let you bypass a SameSite restriction, so if we did that to intentionally bypass SameSite only for the types of requests we care about, I think that would be better for security?
| } | ||
| }; | ||
|
|
||
| let token_endpoint_auth_method = match provider.token_endpoint_auth_method { |
There was a problem hiding this comment.
maybe this would be better suited as an .into() implementation if that's not impossible due to the crates being different
There was a problem hiding this comment.
The crates where both structures are defined don't depend on each other, which is why we can't implement it
sandhose
force-pushed
the
quenting/siwa
branch
from
9ce635a to
04e6960
Compare
For the record, most of it is generic and reusable/useful for other providers! The |
| } else { | ||
| SameSite::Lax | ||
| }); | ||
| cookie.set_same_site(SameSite::Lax); |
There was a problem hiding this comment.
Hi. With this change it doesn't seem to allow auth when element-web (and mas login) are iframed from a different origin.
For example, while developing locally, I have a Vue app at http://localhost:3000. I have element-web running locally via docker at https://foo.bar.
When I access the chat component in my Vue app (where element-web is iframed) I can see the login screen etc jst fine. However, when I POST to the login endpoint, I get a 500 response as the cookie is not honoured with this change in place.
curl 'https://auth.foo.bar/login?kind=continue_authorization_grant&id=01JDSK1J6KSAZER4X4PJG6KKKM' \ -H 'Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,image/apng,*/*;q=0.8,application/signed-exchange;v=b3;q=0.7' \ -H 'Accept-Language: en-GB,en-US;q=0.9,en;q=0.8' \ -H 'Cache-Control: no-cache' \ -H 'Connection: keep-alive' \ -H 'Content-Type: application/x-www-form-urlencoded' \ -H 'Origin: https://auth.foo.bar' \ -H 'Pragma: no-cache' \ -H 'Referer: https://auth.foo.bar/login?kind=continue_authorization_grant&id=01JDSK1J6KSAZER4X4PJG6KKKM' \ -H 'Sec-Fetch-Dest: iframe' \ -H 'Sec-Fetch-Mode: navigate' \ -H 'Sec-Fetch-Site: same-origin' \ -H 'Sec-Fetch-User: ?1' \ -H 'Upgrade-Insecure-Requests: 1' \ -H 'User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/131.0.0.0 Safari/537.36' \ -H 'sec-ch-ua: "Google Chrome";v="131", "Chromium";v="131", "Not_A Brand";v="24"' \ -H 'sec-ch-ua-mobile: ?0' \ -H 'sec-ch-ua-platform: "macOS"' \ --data-raw 'csrf=vrgo4jxAFbaTvA-OKXjX2pMNCFt_rCtC2SCf3G04Yow&username=simon&password=xxxxxxxx'
There was a problem hiding this comment.
Would you happen to know of any workaround?
There was a problem hiding this comment.
This was a temporary change in this PR, which ended up getting reverted. Is the flow working as intended on a known-good client? You should try with:
I'd suggest opening a discussion or joining #matrix-auth:matrix.org for further help on troubleshooting
sandhose
added
the
A-Upstream-OAuth
3 participants
Fixes #2119
This can be reviewed commit-by-commit.
This does three things:
form_postresponse mode