Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix olympus session request error after skipping 2FA Upgrade #21317

Merged
merged 2 commits into from Jul 11, 2023

Conversation

AbbyM
Copy link
Contributor

@AbbyM AbbyM commented Jun 3, 2023

Related issue : #21301

Since last week of 05/2023, requests to : https://appstoreconnect.apple.com/olympus/v1/session are now rejected if account is not configured with 2FA.
Skipping this request avoid non 2FA account to get stuck without authentication.

Checklist

  • I've run bundle exec rspec from the root directory to see all new and existing tests pass
  • I've followed the fastlane code style and run bundle exec rubocop -a to ensure the code style is valid
  • I see several green ci/circleci builds in the "All checks have passed" section of my PR (connect CircleCI to GitHub if not)
  • I've read the Contribution Guidelines
  • I've updated the documentation if necessary.

Motivation and Context

Non 2FA accounts can't use fastlane in CI since 2 weeks now.

Resolves #21301

Description

Comment the request to olympus/session after completing skip 2FA upgrade.

Publish the fix in my git then rebuild my CI docker image pointing fastlane gem to this git and make sure all is working as before.

Testing Steps

Request to : https://appstoreconnect.apple.com/olympus/v1/session are now rejectec if account is not configured with 2FA.
Skipping this request avoid non 2FA account to get stuck without authentication.
@google-cla
Copy link

google-cla bot commented Jun 3, 2023

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

Request to : https://appstoreconnect.apple.com/olympus/v1/session are now rejectec if account is not configured with 2FA.
Skipping this request avoid non 2FA account to get stuck without authentication.
Removing comment for lint
@miketraverso
Copy link

👍

@timsutton
Copy link

We were also running into this error with a non-2FA bot account, and it seems to have resolved the issue! Would love to see if we could get this change merged.

@milch
Copy link
Collaborator

milch commented Jun 9, 2023

Hmm, I was about to merge this since it is currently broken but it seems to throw my application into an infinite loop. I wonder what this olympus session was required for. I also noticed that the key exchange with the server looks completely different now (it does look like SRP but not sure if it is SRP-6a like someone mentioned in #21301), perhaps if that was fixed fetching this olympus session would also continue working properly

@jaysoffian
Copy link
Contributor

I've confirmed this is working on both Enterprise and non-Enterprise accounts using an old login that does NOT have 2FA enabled. It's unfortunate that using a username/password login is still required, but so far the App Store Connect API (which is the only thing that API keys can be used with) is extremely limited. For example, the API does not support renewing APNS certificates. Nor does it support Enterprise accounts at all.

@AbbyM
Copy link
Contributor Author

AbbyM commented Jun 11, 2023

Hmm, I was about to merge this since it is currently broken but it seems to throw my application into an infinite loop. I wonder what this olympus session was required for. I also noticed that the key exchange with the server looks completely different now (it does look like SRP but not sure if it is SRP-6a like someone mentioned in #21301), perhaps if that was fixed fetching this olympus session would also continue working properly

Hi,

This fix should impact only non-2FA accounts.

Even with a normal login through Apple website / skip 2FA upgrade , you can't get anymore an olympus session.

Check this post : #21301 (comment)

If found a particular usage of this function in

# We use the olympus session to determine if the old session is still valid
:

          # We use the olympus session to determine if the old session is still valid
          # As this will raise an exception if the old session has expired
          # If the old session is still valid, we don't have to do anything else in this method
          # that's why we return true

This is small :

def fetch_olympus_session

# Get the `itctx` from the new (22nd May 2017) API endpoint "olympus"
# Update (29th March 2019) olympus migrates to new appstoreconnect API
    
    def fetch_olympus_session
      response = request(:get, "https://appstoreconnect.apple.com/olympus/v1/session")
      body = response.body
      if body
        body = JSON.parse(body) if body.kind_of?(String)
        user_map = body["user"]
        if user_map
          self.user_email = user_map["emailAddress"]
        end

        provider = body["provider"]
        if provider
          self.provider = Spaceship::Provider.new(provider_hash: provider)
          return true
        end
      end

      return false
    end

Maybe this line have impacts :

self.provider = Spaceship::Provider.new(provider_hash: provider)

@timsutton
Copy link

re:

so far the App Store Connect API (which is the only thing that API keys can be used with) is extremely limited

I recommend to file feedback to Apple if you haven't already. You can reference these FBs:

  • FB9161884
  • FB7339579
  • FB8989082

@justinseanmartin
Copy link

Also wanted to add a note that we're using this branch currently to sidestep the ADC issues with the latest published version of Fastlane, and has been working great for us.

@milch
Copy link
Collaborator

milch commented Jul 11, 2023

My team has been able to test this again, and it seems to be working now. Not sure what happened earlier. Anyway, given that there have been multiple confirmations that it is working, and I see several forks have been created, it probably makes the most sense to merge!

@milch milch merged commit 3d2d208 into fastlane:master Jul 11, 2023
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Did Apple break SPACESHIP_SKIP_2FA_UPGRADE=1?
6 participants