Skip to content

Update Grape::Middleware::Auth::Base #2563

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

Merged
merged 1 commit into from
May 5, 2025

Conversation

ericproulx
Copy link
Contributor

@ericproulx ericproulx commented May 4, 2025

Grape::Middleware::Auth::Base inherits from Grape::Middleware::Base and its type is now validated at compile time instead of runtime. type is static and it should be known at compile time that is wrong.

@ericproulx ericproulx requested a review from Copilot May 4, 2025 19:19
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR updates the authentication middleware by enforcing validation of the auth strategy at compile time and by refactoring the middleware base class and DSL modules. Key changes include:

  • Updating Grape::Middleware::Auth::Base to inherit from Grape::Middleware::Base and raising a custom exception when an unknown auth strategy is provided.
  • Removing ActiveSupport::Concern and associated DSL extensions in Grape::Middleware::Auth::DSL.
  • Updating error messages in the locale file and adjusting documentation accordingly.

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.

Show a summary per file
File Description
spec/grape/middleware/auth/strategies_spec.rb Added tests to verify a 401 error is thrown for unknown auth strategies
lib/grape/middleware/auth/dsl.rb Removed ActiveSupport::Concern usage and the ClassMethods block
lib/grape/middleware/auth/base.rb Refactored to inherit from Grape::Middleware::Base and updated auth strategy handling
lib/grape/locale/en.yml Updated error message formats and key names
lib/grape/exceptions/unknown_auth_strategy.rb Added a custom exception for unknown auth strategies
lib/grape/dsl/api.rb Removed inclusion of the auth DSL module
lib/grape/api/instance.rb Extended auth DSL to support the middleware updates
UPGRADING.md Documented changes related to authentication middleware and API behavior updates
Comments suppressed due to low confidence (2)

lib/grape/middleware/auth/dsl.rb:4

  • [nitpick] Removing ActiveSupport::Concern and the ClassMethods block may affect how extensions are handled; consider verifying that this change does not disrupt the intended DSL behavior in consumer classes.
module Grape::Middleware::Auth::DSL

lib/grape/middleware/auth/base.rb:17

  • The use of 'context' is unclear since it is not defined in this snippet; please confirm that it is provided by Grape::Middleware::Base or use a more explicit reference.
context.instance_exec(*args, &options[:proc])

Grape::Middleware::Auth::DSL is not a concern
en.yml sorted
`options[:type]` is now validated at compile time instead of runtime
Grape::Middleware::Auth::Base raises an exception instead of throw
Add UPGRADING.md notes
@ericproulx ericproulx force-pushed the revisit_auth_middleware branch from 706f14f to 615cdb2 Compare May 4, 2025 19:22
@ericproulx ericproulx marked this pull request as ready for review May 4, 2025 19:22
@ericproulx ericproulx requested a review from dblock May 4, 2025 19:22
@dblock dblock merged commit 10128ad into ruby-grape:master May 5, 2025
63 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.

2 participants