Skip to content

Conversation

rsmithlal
Copy link
Member

Summary

Describe the change and the motivation.

Checklist

  • Tests added/updated and passing (bin/ci).
  • Lint and security checks (rubocop, brakeman, bundler-audit).
  • Documentation updated under docs/ describing new/changed functionality.
  • Mermaid diagrams (docs/*.mmd) updated to reflect changes.
  • Rendered PNGs regenerated with bin/render_diagrams and committed.
  • For DB changes, included any needed backfills/dedupes and noted risks.

Screenshots / Diagrams

If applicable, include screenshots or link to updated diagrams.

Notes

Anything reviewers should be aware of (migration order, flags, feature toggles).

use BetterTogether controller
Prevent visibility of the feature before it's ready
Allows for linking OAuth authorizations to Person and Platform records
- Introduced new spec for Simple OAuth Flow in `oauth_simple_spec.rb` to validate user creation and integration handling.
- Enhanced `person_platform_integration_spec.rb` with tests for attributes extraction, token management, and integration updates.
- Created `devise_user_spec.rb` to test user creation from OAuth and attribute setting from auth hash.
- Added support helpers in `oauth_test_helpers.rb` for generating mock OAuth auth hashes for various providers.
- Implemented shared examples in `oauth_examples.rb` for consistent testing of OAuth authentication flows and token management.
@@ -0,0 +1,59 @@
class BetterTogether::OmniauthCallbacksController < Devise::OmniauthCallbacksController
# See https://github.com/omniauth/omniauth/wiki/FAQ#rails-session-is-clobbered-after-callback-on-developer-strategy
skip_before_action :verify_authenticity_token, only: %i[github]

Check failure

Code scanning / CodeQL

CSRF protection weakened or disabled High

Potential CSRF vulnerability due to forgery protection being disabled or weakened.

Copilot Autofix

AI 27 days ago

To fix this issue, re-enable CSRF protection for the github callback by removing or altering the line that skips CSRF verification for it. The best, most minimal fix is to delete or comment out the skip_before_action :verify_authenticity_token, only: %i[github] line (line 3 in the given code). This restores Rails' default (and secure) behaviour: requests made to the github callback will require the authenticity token, mitigating CSRF risk. No additional code or imports are needed—just remove that single line.

Suggested changeset 1
app/controllers/better_together/omniauth_callbacks_controller.rb

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/app/controllers/better_together/omniauth_callbacks_controller.rb b/app/controllers/better_together/omniauth_callbacks_controller.rb
--- a/app/controllers/better_together/omniauth_callbacks_controller.rb
+++ b/app/controllers/better_together/omniauth_callbacks_controller.rb
@@ -1,6 +1,5 @@
 class BetterTogether::OmniauthCallbacksController < Devise::OmniauthCallbacksController
   # See https://github.com/omniauth/omniauth/wiki/FAQ#rails-session-is-clobbered-after-callback-on-developer-strategy
-  skip_before_action :verify_authenticity_token, only: %i[github]
 
   before_action :set_person_platform_integration, except: [:failure]
   before_action :set_user, except: [:failure]
EOF
@@ -1,6 +1,5 @@
class BetterTogether::OmniauthCallbacksController < Devise::OmniauthCallbacksController
# See https://github.com/omniauth/omniauth/wiki/FAQ#rails-session-is-clobbered-after-callback-on-developer-strategy
skip_before_action :verify_authenticity_token, only: %i[github]

before_action :set_person_platform_integration, except: [:failure]
before_action :set_user, except: [:failure]
Copilot is powered by AI and may make mistakes. Always verify output.
@@ -0,0 +1,10 @@
<p style="color: green"><%= notice %></p>

<%= render @person_platform_integration %>

Check notice

Code scanning / Brakeman

Render path contains parameter value. Note

Render path contains parameter value.
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.

1 participant