Sign in with Apple

Will Sign in with Apple be a supported sign in method when it reaches public availability later this year?

16 Likes

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.

12 Likes

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? :wink:

I also feel itā€™s important to support identity management with a company that is more privacy focussed that, ahem, others we could mention.

13 Likes

Do it @merefield!

As there is a omniauth-apple strategy already it should be doable https://github.com/nhosoya/omniauth-apple

16 Likes

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?

1 Like

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.

7 Likes

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!

7 Likes

Maybe we use a file upload for the pem file? How large is it?

Itā€™s tiny :slight_smile:

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 :slight_smile:

8 Likes

@merefield I have it successfully running on sandbox.dtaylor.uk :tada:

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:

11 Likes

Thatā€™s great! Ah textarea is new to me. That avoids my ā€˜hackā€™ I added nicely. Perfect! If only I had known about that! :sweat_smile:

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! :wink:

7 Likes

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 :tada:

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ā€™ve commented on it

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

5 Likes

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.

4 Likes