Will Sign in with Apple be a supported sign in method when it reaches public availability later this year?
Nobodyās been assigned to it yet but I suspect itāll happen if they gain traction. Even if we donāt do it quickly in discourse core I think itās likely someone in the community will create a plugin.
Cool. I hadnāt seen it mentioned anywhere yet, so thought Iād bring it up!
I could have a crack at this, given I just PRād to the Discord one ā¦ how hard can it be?
I also feel itās important to support identity management with a company that is more privacy focussed that, ahem, others we could mention.
Do it @merefield!
As there is a omniauth-apple strategy already it should be doable https://github.com/nhosoya/omniauth-apple
Very useful, thanks Rafael
Very close wtih this now, but have encountered a snag:
The OAuth strategy requires a pem
key file (which you get from those kind people at Apple)
When I attempt to store this in a SiteSetting, the text is somehow corrupted and generation of the private key fails:
::OpenSSL::PKey::EC.new(options.pem)
# OpenSSL::PKey::ECError
## invalid curve name
Iāve attempted to escape the text with \n
's where there are new lines.
I canāt see how this can be deployable unless we can somehow store the contents of this file in a SiteSetting?
The .pem is just the public key, isnāt it?
In this case it includes the private key (apparently there are other things encoded in it).
The code goes on to use this private key to generate the client secret.
Iāve tested the library with the pem file and itās valid, but as soon as you paste the file into a SiteSetting it (perhaps predictably) getās subtly altered.
UPDATE: looks like \n
is replaced with \\n
in SiteSettings, so may be able to search for \\
on retrieve and cut it down by one
Seems like I was able to deal, sorry for the spam.
An update:
My plugin appears to be working up to a point, however, Iām currently getting a vague error from Apple when attempting to authenticate with the credentials Iāve set up as an Apple Developer:
āYour request could not be completed because of an error. Please try again laterā
As you might imagine, this doesnāt give me a lot to go on.
This is compounded slightly because their authorization scheme is a lot different to the OAuth 2.0 standard.
Unfortunately Iām not able to raise a full TSI ticket with Apple yet because the feature is pre-release, but I will submit a Feedback Issue.
The repo is here:
https://github.com/merefield/discourse-sign-in-with-apple
NB Use at your own risk - not yet successfully tested!
Maybe we use a file upload for the pem
file? How large is it?
Itās tiny
PEM āfileā (as a string in Site Setttings) seems to be processed fine, so that doesnāt seem to be the issue any longer.
That original error has gone away. JWT seems to process it fine now.
I solved it by replacing the new lines with a $ sign, and then replacing the $ sign with a new line when passing it to the JWT function. Non-standard, but works. Supplying ā/ā messes things up because of the way Ruby handles it. (I admit however, though no exception is raised, this could still be a problem area)
You are more than welcome to use your Apple credentials and give it a go.
I fear Iāve got something wrong with the credentials.
You need to provide:
- Team ID
- Client ID
- PEM
- Key ID
Itās very fiddly to set this all up on Appleās website
@merefield I have it successfully running on sandbox.dtaylor.uk
I had to make a couple of changes in our fork to allow for domain verification. I also added some more specific descriptions to the site settings, and enabled multi-line support for the PEM.
It looks like the omniauth-apple gem does not (yet?) support passing any information about the userā¦ which is not particularly useful for us. It looks like sign-in-with-apple is almost OpenID-Connect compatible, so itās possible we might be able to re-use some of the functionality from that plugin.
In terms of setting up the credentials, I found this (very appropriately named) blog post far more useful than Appleās documentation:
Thatās great! Ah textarea
is new to me. That avoids my āhackā I added nicely. Perfect! If only I had known about that!
I like the txt file verification check you added, nice touch. I had done that manually, but thatās a great assistance for deployed use.
Yes I used that too.
Sadly Iām still getting the same Apple-side error after merging your fork, so I suspect Iām still doing something silly with credentials. I may PM you if I may to swap notes!
OK, turned out my problem was almost certainly trying to access with a partially dev site (running with nginx and over https though).
I retried with a Production site and
but unfortunately, as you say, āNameā is not returned and this must be the middleware, right? As looks like Apple letting you authorise this.
Are we sure that it will return a name? From a privacy perspective itās almost better if the user picks a name than have any public PII disclosed in the process.
It should do technically? But agree with your point, though you can choose not to expose it at the Apple page. Moot, as it turns out so far!
Yes, someone had raised an issue about this, but it was then closed.
Iām guessing this is the area of code weāre concerned about?:
info do
{ sub: id_token['sub'] }
end
whereas in the discord auth gem, for example, we get this:
info do
{
name: raw_info['username'],
email: raw_info['verified'] ? raw_info['email'] : nil,
image: "https://cdn.discordapp.com/avatars/#{raw_info['id']}/#{raw_info['avatar']}"
}
end
FYI No mention of those fields in Appleās API documentation ā¦
It looks like this PR adds the useful user information: https://github.com/nhosoya/omniauth-apple/pull/7. Hopefully it will be merged soon, or if not we could look at using a fork
Yes, you are right, Iād seen that PR but not dug into the code and thought it was simply switching to a different dependency. People should consider not PR-ing with such huge scope as details like that can be lost.
As you said:
{
sub: id_info['sub'],
email: user_info.dig('email'),
first_name: user_info.dig('name', 'firstName'),
last_name: user_info.dig('name', 'lastName'),
extra: {
raw_info: id_info.merge(user_info)
}
}
Iāve added a comment to support the PR.