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.

Sign up for GitHub

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

Merge changes into official CoinGecko repo #1

Merged
merged 2 commits into from
Sep 11, 2024
Merged

Merge changes into official CoinGecko repo #1

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d277c73
Update apple.rb
wenjieyek Dec 6, 2023
3d78527
add callback phase
wenjieyek Dec 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion lib/omniauth/strategies/apple.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,20 @@ def callback_url
options[:redirect_uri] || (full_host + callback_path)
end

def callback_phase
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need this callback_phase? What does it actually do? 🤔

if request.request_method.downcase.to_sym == :post
Copy link
Collaborator Author

@ervinchai ervinchai Jul 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like we're solving this; https://developer.apple.com/documentation/sign_in_with_apple/sign_in_with_apple_js/configuring_your_webpage_for_sign_in_with_apple#3331292

url = "#{callback_url}"
if (code = request.params['code']) && (state = request.params['state'])
url += "?code=#{CGI::escape(code)}"
url += "&state=#{CGI::escape(state)}"
url += "&user=#{CGI::escape(request.params['user'])}" if request.params['user']
end
Comment on lines +68 to +73
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we attaching these params on the query params of the callback URI? 🤔

session.options[:drop] = true # Do not set a session cookie on this response
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why?

return redirect url
end
super
end

private

def new_nonce
Expand Down Expand Up @@ -105,7 +119,7 @@ def verify_claims!(id_token)
verify_aud!(id_token)
verify_iat!(id_token)
verify_exp!(id_token)
verify_nonce!(id_token) if id_token[:nonce_supported]
# verify_nonce!(id_token) if id_token[:nonce_supported]
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to figure out why this is done. We really shouldn't be disabling the nonce verification, as it would allow for replay attacks.

end

def verify_iss!(id_token)
Expand Down