-
Notifications
You must be signed in to change notification settings - Fork 5
1004: Add new endpoint for google token exchange #609
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,45 @@ | ||||||||||||||||
| # frozen_string_literal: true | ||||||||||||||||
|
|
||||||||||||||||
| module Api | ||||||||||||||||
| class GoogleAuthController < ApiController | ||||||||||||||||
| TOKEN_EXCHANGE_URL = 'https://oauth2.googleapis.com/token' | ||||||||||||||||
|
|
||||||||||||||||
| before_action :authorize_user | ||||||||||||||||
| authorize_resource :google_auth, class: false | ||||||||||||||||
|
|
||||||||||||||||
| def exchange_code | ||||||||||||||||
| payload = google_token_params | ||||||||||||||||
|
|
||||||||||||||||
| request_body = { | ||||||||||||||||
| code: payload[:code], | ||||||||||||||||
| client_id: ENV.fetch('GOOGLE_CLIENT_ID'), | ||||||||||||||||
| client_secret: ENV.fetch('GOOGLE_CLIENT_SECRET'), | ||||||||||||||||
| redirect_uri: payload[:redirect_uri], | ||||||||||||||||
| grant_type: 'authorization_code' | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| conn = Faraday.new do |f| | ||||||||||||||||
| f.request :url_encoded | ||||||||||||||||
| end | ||||||||||||||||
|
|
||||||||||||||||
| response = conn.post(TOKEN_EXCHANGE_URL, request_body) | ||||||||||||||||
| @token_response = JSON.parse(response.body) | ||||||||||||||||
|
||||||||||||||||
| @token_response = JSON.parse(response.body) | |
| begin | |
| @token_response = JSON.parse(response.body) | |
| rescue JSON::ParserError => e | |
| render json: { error: "Invalid response from Google: #{e.message}" }, status: :bad_gateway | |
| return | |
| end |
Copilot
AI
Nov 13, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential nil error message. If Google returns an error response without an error_description field, @token_response['error_description'] will be nil, resulting in { error: nil } being returned to the client. This could be confusing for API consumers.
Consider providing a fallback error message or using @token_response['error'] as a fallback if error_description is not present.
| render json: { error: @token_response['error_description'] }, status: :unauthorized | |
| error_message = @token_response['error_description'] || @token_response['error'] || 'Unknown error' | |
| render json: { error: error_message }, status: :unauthorized |
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,10 @@ | ||||||||
| # frozen_string_literal: true | ||||||||
|
|
||||||||
| json.call( | ||||||||
| @token_response, | ||||||||
| 'access_token', | ||||||||
| 'expires_in', | ||||||||
| 'token_type', | ||||||||
| 'scope', | ||||||||
| 'id_token' | ||||||||
|
||||||||
| 'id_token' | |
| 'id_token', | |
| 'refresh_token' |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,184 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| require 'rails_helper' | ||
|
|
||
| RSpec.describe 'Google Auth requests' do | ||
| let(:headers) { { Authorization: UserProfileMock::TOKEN } } | ||
| let(:school) { create(:school) } | ||
| let(:owner) { create(:owner, school:) } | ||
|
|
||
| before do | ||
| authenticated_in_hydra_as(owner) | ||
| end | ||
|
|
||
| describe 'POST /api/google_auth/exchange_code' do | ||
| let(:params) do | ||
| { | ||
| google_auth: { | ||
| code: 'test-authorization-code', | ||
| redirect_uri: 'https://example.com/callback' | ||
| } | ||
| } | ||
| end | ||
|
|
||
| let(:google_token_response) do | ||
| { | ||
| 'access_token' => 'test-access-token', | ||
| 'expires_in' => 3599, | ||
| 'token_type' => 'Bearer', | ||
| 'scope' => 'openid email profile', | ||
| 'id_token' => 'test-id-token' | ||
| } | ||
| end | ||
|
|
||
| around do |example| | ||
| ClimateControl.modify( | ||
| GOOGLE_CLIENT_ID: 'test-client-id', | ||
| GOOGLE_CLIENT_SECRET: 'test-client-secret' | ||
| ) do | ||
| example.run | ||
| end | ||
| end | ||
|
|
||
| context 'when token exchange is successful' do | ||
| before do | ||
| stub_request(:post, Api::GoogleAuthController::TOKEN_EXCHANGE_URL) | ||
| .with( | ||
| body: { | ||
| code: 'test-authorization-code', | ||
| client_id: 'test-client-id', | ||
| client_secret: 'test-client-secret', | ||
| redirect_uri: 'https://example.com/callback', | ||
| grant_type: 'authorization_code' | ||
| } | ||
| ) | ||
| .to_return( | ||
| status: 200, | ||
| body: google_token_response.to_json, | ||
| headers: { 'Content-Type' => 'application/json' } | ||
| ) | ||
| end | ||
|
|
||
| it 'returns success response' do | ||
| post('/api/google/auth/exchange-code', params:, headers:) | ||
| expect(response).to have_http_status(:ok) | ||
| end | ||
|
|
||
| it 'returns token response from Google' do | ||
| post('/api/google/auth/exchange-code', params:, headers:) | ||
| expect(response.parsed_body).to eq(google_token_response) | ||
| end | ||
|
|
||
| it 'includes access_token in response' do | ||
| post('/api/google/auth/exchange-code', params:, headers:) | ||
| expect(response.parsed_body['access_token']).to eq('test-access-token') | ||
| end | ||
| end | ||
|
|
||
| context 'when token exchange fails with error from Google' do | ||
| let(:error_response) do | ||
| { | ||
| 'error' => 'invalid_grant', | ||
| 'error_description' => 'Bad Request' | ||
| } | ||
| end | ||
|
|
||
| before do | ||
| stub_request(:post, Api::GoogleAuthController::TOKEN_EXCHANGE_URL) | ||
| .to_return( | ||
| status: 400, | ||
| body: error_response.to_json, | ||
| headers: { 'Content-Type' => 'application/json' } | ||
| ) | ||
| end | ||
|
|
||
| it 'returns unauthorized response' do | ||
| post('/api/google/auth/exchange-code', params:, headers:) | ||
| expect(response).to have_http_status(:unauthorized) | ||
| end | ||
|
|
||
| it 'returns error message' do | ||
| post('/api/google/auth/exchange-code', params:, headers:) | ||
| expect(response.parsed_body['error']).to eq('Bad Request') | ||
| end | ||
| end | ||
|
|
||
| context 'when network error occurs' do | ||
| before do | ||
| stub_request(:post, Api::GoogleAuthController::TOKEN_EXCHANGE_URL) | ||
| .to_raise(Faraday::ConnectionFailed.new('Connection failed')) | ||
| end | ||
|
|
||
| it 'returns service unavailable response' do | ||
| post('/api/google/auth/exchange-code', params:, headers:) | ||
| expect(response).to have_http_status(:service_unavailable) | ||
| end | ||
|
|
||
| it 'returns error message' do | ||
| post('/api/google/auth/exchange-code', params:, headers:) | ||
| expect(response.parsed_body['error']).to eq('Connection failed') | ||
| end | ||
| end | ||
|
|
||
| context 'when code parameter is missing' do | ||
| let(:params) do | ||
| { | ||
| google_auth: { | ||
| redirect_uri: 'https://example.com/callback' | ||
| } | ||
| } | ||
| end | ||
|
|
||
| it 'returns bad request response' do | ||
| post('/api/google/auth/exchange-code', params:, headers:) | ||
| expect(response).to have_http_status(:bad_request) | ||
| end | ||
| end | ||
|
|
||
| context 'when redirect_uri parameter is missing' do | ||
| let(:params) do | ||
| { | ||
| google_auth: { | ||
| code: 'test-authorization-code' | ||
| } | ||
| } | ||
| end | ||
|
|
||
| it 'returns bad request response' do | ||
| post('/api/google/auth/exchange-code', params:, headers:) | ||
| expect(response).to have_http_status(:bad_request) | ||
| end | ||
| end | ||
|
|
||
| context 'when google_auth params are missing' do | ||
| it 'returns bad request response' do | ||
| post('/api/google/auth/exchange-code', headers:) | ||
| expect(response).to have_http_status(:bad_request) | ||
| end | ||
| end | ||
|
|
||
| context 'when user is not authenticated' do | ||
| before do | ||
| unauthenticated_in_hydra | ||
| end | ||
|
|
||
| it 'returns unauthorized response' do | ||
| post('/api/google/auth/exchange-code', params:, headers:) | ||
| expect(response).to have_http_status(:unauthorized) | ||
| end | ||
| end | ||
|
|
||
| context 'when user is not authorized' do | ||
| let(:student) { create(:student, school:) } | ||
|
|
||
| before do | ||
| authenticated_in_hydra_as(student) | ||
| end | ||
|
|
||
| it 'returns forbidden response' do | ||
| post('/api/google/auth/exchange-code', params:, headers:) | ||
| expect(response).to have_http_status(:forbidden) | ||
| end | ||
| end | ||
| end | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] No timeout configuration for external API call. While Faraday has default timeouts, it's a best practice to explicitly configure timeouts for external API calls to prevent long-running requests from blocking server resources.
Consider adding explicit timeout configuration: