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

nil dereference in api/v1/authorisations#create when a named permission doesn't exist #1824

Open
sengi opened this issue Mar 28, 2022 · 1 comment

Comments

@sengi
Copy link
Contributor

sengi commented Mar 28, 2022

Repro: send a POST request to /api/v1/api-users/14149/authorisations (any valid api-user id will do; this one is valid in integration) with the body {"application_id":44,"permissions":["nonexistent_permission"]} (any valid application_id will do). You'll need a valid token to put in the Authorization: bearer ... header.

Expected behaviour: error response saying that the nonexistent_permission permission doesn't exist on application 44 (or preferably whatever the human-friendly name is for that app).

Observed behaviour: 500 response with an HTML body, and a stack trace in the log.

{
  "exception_class": "NoMethodError",
  "exception_message": "undefined method `id' for nil:NilClass",
  "stacktrace": [
    "app/models/user.rb:148:in `grant_permission'",
    "app/models/user.rb:143:in `block in grant_application_permissions'",
    "app/models/user.rb:141:in `map'",
    "app/models/user.rb:141:in `grant_application_permissions'",
    "app/controllers/api/v1/authorisations_controller.rb:54:in `grant_app_permissions!'",
    "app/controllers/api/v1/authorisations_controller.rb:14:in `block in create'",
    "app/controllers/api/v1/authorisations_controller.rb:12:in `create'",
    "lib/same_site_security/middleware.rb:9:in `call'"
  ]
}

Full stack trace:

NoMethodError
undefined method `id' for nil:NilClass
/app/app/models/user.rb:148:in `grant_permission'
/app/app/models/user.rb:143:in `block in grant_application_permissions'
/app/app/models/user.rb:141:in `map'
/app/app/models/user.rb:141:in `grant_application_permissions'
/app/app/controllers/api/v1/authorisations_controller.rb:54:in `grant_app_permissions!'
/app/app/controllers/api/v1/authorisations_controller.rb:14:in `block in create'
/app/vendor/bundle/ruby/2.7.0/gems/activerecord-6.1.5/lib/active_record/connection_adapters/abstract/database_statements.rb:320:in `block in transaction'
/app/vendor/bundle/ruby/2.7.0/gems/activerecord-6.1.5/lib/active_record/connection_adapters/abstract/transaction.rb:319:in `block in within_new_transaction'
/app/vendor/bundle/ruby/2.7.0/gems/activesupport-6.1.5/lib/active_support/concurrency/load_interlock_aware_monitor.rb:26:in `block (2 levels) in synchronize'
/app/vendor/bundle/ruby/2.7.0/gems/activesupport-6.1.5/lib/active_support/concurrency/load_interlock_aware_monitor.rb:25:in `handle_interrupt'
/app/vendor/bundle/ruby/2.7.0/gems/activesupport-6.1.5/lib/active_support/concurrency/load_interlock_aware_monitor.rb:25:in `block in synchronize'
/app/vendor/bundle/ruby/2.7.0/gems/activesupport-6.1.5/lib/active_support/concurrency/load_interlock_aware_monitor.rb:21:in `handle_interrupt'
/app/vendor/bundle/ruby/2.7.0/gems/activesupport-6.1.5/lib/active_support/concurrency/load_interlock_aware_monitor.rb:21:in `synchronize'
/app/vendor/bundle/ruby/2.7.0/gems/activerecord-6.1.5/lib/active_record/connection_adapters/abstract/transaction.rb:317:in `within_new_transaction'
/app/vendor/bundle/ruby/2.7.0/gems/activerecord-6.1.5/lib/active_record/connection_adapters/abstract/database_statements.rb:320:in `transaction'
/app/vendor/bundle/ruby/2.7.0/gems/activerecord-6.1.5/lib/active_record/transactions.rb:209:in `transaction'
/app/app/controllers/api/v1/authorisations_controller.rb:12:in `create'
/app/vendor/bundle/ruby/2.7.0/gems/actionpack-6.1.5/lib/action_controller/metal/basic_implicit_render.rb:6:in `send_action'
/app/vendor/bundle/ruby/2.7.0/gems/actionpack-6.1.5/lib/abstract_controller/base.rb:228:in `process_action'
/app/vendor/bundle/ruby/2.7.0/gems/actionpack-6.1.5/lib/action_controller/metal/rendering.rb:30:in `process_action'
/app/vendor/bundle/ruby/2.7.0/gems/actionpack-6.1.5/lib/abstract_controller/callbacks.rb:42:in `block in process_action'
/app/vendor/bundle/ruby/2.7.0/gems/activesupport-6.1.5/lib/active_support/callbacks.rb:106:in `run_callbacks'
/app/vendor/bundle/ruby/2.7.0/gems/actionpack-6.1.5/lib/abstract_controller/callbacks.rb:41:in `process_action'
/app/vendor/bundle/ruby/2.7.0/gems/actionpack-6.1.5/lib/action_controller/metal/rescue.rb:22:in `process_action'
/app/vendor/bundle/ruby/2.7.0/gems/logstasher-2.1.5/lib/logstasher/rails_ext/action_controller/metal/instrumentation.rb:40:in `block in process_action'
/app/vendor/bundle/ruby/2.7.0/gems/activesupport-6.1.5/lib/active_support/notifications.rb:203:in `block in instrument'
/app/vendor/bundle/ruby/2.7.0/gems/activesupport-6.1.5/lib/active_support/notifications/instrumenter.rb:24:in `instrument'
/app/vendor/bundle/ruby/2.7.0/gems/activesupport-6.1.5/lib/active_support/notifications.rb:203:in `instrument'
/app/vendor/bundle/ruby/2.7.0/gems/logstasher-2.1.5/lib/logstasher/rails_ext/action_controller/metal/instrumentation.rb:27:in `process_action'
/app/vendor/bundle/ruby/2.7.0/gems/actionpack-6.1.5/lib/action_controller/metal/params_wrapper.rb:249:in `process_action'
/app/vendor/bundle/ruby/2.7.0/gems/activerecord-6.1.5/lib/active_record/railties/controller_runtime.rb:27:in `process_action'
/app/vendor/bundle/ruby/2.7.0/gems/actionpack-6.1.5/lib/abstract_controller/base.rb:165:in `process'
/app/vendor/bundle/ruby/2.7.0/gems/actionview-6.1.5/lib/action_view/rendering.rb:39:in `process'
/app/vendor/bundle/ruby/2.7.0/gems/actionpack-6.1.5/lib/action_controller/metal.rb:190:in `dispatch'
/app/vendor/bundle/ruby/2.7.0/gems/actionpack-6.1.5/lib/action_controller/metal.rb:254:in `dispatch'
/app/vendor/bundle/ruby/2.7.0/gems/actionpack-6.1.5/lib/action_dispatch/routing/route_set.rb:50:in `dispatch'
/app/vendor/bundle/ruby/2.7.0/gems/actionpack-6.1.5/lib/action_dispatch/routing/route_set.rb:33:in `serve'
/app/vendor/bundle/ruby/2.7.0/gems/actionpack-6.1.5/lib/action_dispatch/journey/router.rb:50:in `block in serve'
/app/vendor/bundle/ruby/2.7.0/gems/actionpack-6.1.5/lib/action_dispatch/journey/router.rb:32:in `each'
/app/vendor/bundle/ruby/2.7.0/gems/actionpack-6.1.5/lib/action_dispatch/journey/router.rb:32:in `serve'
/app/vendor/bundle/ruby/2.7.0/gems/actionpack-6.1.5/lib/action_dispatch/routing/route_set.rb:842:in `call'
/app/vendor/bundle/ruby/2.7.0/gems/sentry-rails-4.5.2/lib/sentry/rails/rescued_exception_interceptor.rb:12:in `call'
/app/vendor/bundle/ruby/2.7.0/gems/gds-api-adapters-79.1.0/lib/gds_api/middleware/govuk_header_sniffer.rb:12:in `call'
/app/vendor/bundle/ruby/2.7.0/gems/gds-api-adapters-79.1.0/lib/gds_api/middleware/govuk_header_sniffer.rb:12:in `call'
/app/vendor/bundle/ruby/2.7.0/gems/gds-api-adapters-79.1.0/lib/gds_api/middleware/govuk_header_sniffer.rb:12:in `call'
/app/vendor/bundle/ruby/2.7.0/gems/gds-api-adapters-79.1.0/lib/gds_api/middleware/govuk_header_sniffer.rb:12:in `call'
/app/vendor/bundle/ruby/2.7.0/gems/gds-api-adapters-79.1.0/lib/gds_api/middleware/govuk_header_sniffer.rb:12:in `call'
/app/vendor/bundle/ruby/2.7.0/gems/rack-attack-6.5.0/lib/rack/attack.rb:113:in `call'
/app/vendor/bundle/ruby/2.7.0/gems/warden-1.2.9/lib/warden/manager.rb:36:in `block in call'
/app/vendor/bundle/ruby/2.7.0/gems/warden-1.2.9/lib/warden/manager.rb:34:in `catch'
/app/vendor/bundle/ruby/2.7.0/gems/warden-1.2.9/lib/warden/manager.rb:34:in `call'
/app/vendor/bundle/ruby/2.7.0/gems/rack-2.2.3/lib/rack/tempfile_reaper.rb:15:in `call'
/app/vendor/bundle/ruby/2.7.0/gems/rack-2.2.3/lib/rack/etag.rb:27:in `call'
/app/vendor/bundle/ruby/2.7.0/gems/rack-2.2.3/lib/rack/conditional_get.rb:40:in `call'
/app/vendor/bundle/ruby/2.7.0/gems/rack-2.2.3/lib/rack/head.rb:12:in `call'
/app/vendor/bundle/ruby/2.7.0/gems/actionpack-6.1.5/lib/action_dispatch/http/permissions_policy.rb:22:in `call'
/app/vendor/bundle/ruby/2.7.0/gems/actionpack-6.1.5/lib/action_dispatch/http/content_security_policy.rb:19:in `call'
/app/vendor/bundle/ruby/2.7.0/gems/rack-2.2.3/lib/rack/session/abstract/id.rb:266:in `context'
/app/vendor/bundle/ruby/2.7.0/gems/rack-2.2.3/lib/rack/session/abstract/id.rb:260:in `call'
/app/vendor/bundle/ruby/2.7.0/gems/actionpack-6.1.5/lib/action_dispatch/middleware/cookies.rb:689:in `call'
/app/vendor/bundle/ruby/2.7.0/gems/actionpack-6.1.5/lib/action_dispatch/middleware/callbacks.rb:27:in `block in call'
/app/vendor/bundle/ruby/2.7.0/gems/activesupport-6.1.5/lib/active_support/callbacks.rb:98:in `run_callbacks'
/app/vendor/bundle/ruby/2.7.0/gems/actionpack-6.1.5/lib/action_dispatch/middleware/callbacks.rb:26:in `call'
/app/vendor/bundle/ruby/2.7.0/gems/actionpack-6.1.5/lib/action_dispatch/middleware/actionable_exceptions.rb:18:in `call'
/app/vendor/bundle/ruby/2.7.0/gems/actionpack-6.1.5/lib/action_dispatch/middleware/debug_exceptions.rb:29:in `call'
/app/vendor/bundle/ruby/2.7.0/gems/actionpack-6.1.5/lib/action_dispatch/middleware/show_exceptions.rb:33:in `call'
/app/vendor/bundle/ruby/2.7.0/gems/logstasher-2.1.5/lib/logstasher/rails_ext/rack/logger.rb:16:in `call_app'
/app/vendor/bundle/ruby/2.7.0/gems/railties-6.1.5/lib/rails/rack/logger.rb:26:in `block in call'
/app/vendor/bundle/ruby/2.7.0/gems/activesupport-6.1.5/lib/active_support/tagged_logging.rb:99:in `block in tagged'
/app/vendor/bundle/ruby/2.7.0/gems/activesupport-6.1.5/lib/active_support/tagged_logging.rb:37:in `tagged'
/app/vendor/bundle/ruby/2.7.0/gems/activesupport-6.1.5/lib/active_support/tagged_logging.rb:99:in `tagged'
/app/vendor/bundle/ruby/2.7.0/gems/railties-6.1.5/lib/rails/rack/logger.rb:26:in `call'
/app/vendor/bundle/ruby/2.7.0/gems/actionpack-6.1.5/lib/action_dispatch/middleware/remote_ip.rb:81:in `call'
/app/vendor/bundle/ruby/2.7.0/gems/request_store-1.5.1/lib/request_store/middleware.rb:19:in `call'
/app/vendor/bundle/ruby/2.7.0/gems/actionpack-6.1.5/lib/action_dispatch/middleware/request_id.rb:26:in `call'
/app/vendor/bundle/ruby/2.7.0/gems/rack-2.2.3/lib/rack/method_override.rb:24:in `call'
/app/vendor/bundle/ruby/2.7.0/gems/rack-2.2.3/lib/rack/runtime.rb:22:in `call'
/app/vendor/bundle/ruby/2.7.0/gems/activesupport-6.1.5/lib/active_support/cache/strategy/local_cache_middleware.rb:29:in `call'
/app/vendor/bundle/ruby/2.7.0/gems/sentry-ruby-core-4.5.2/lib/sentry/rack/capture_exceptions.rb:23:in `block in call'
/app/vendor/bundle/ruby/2.7.0/gems/sentry-ruby-core-4.5.2/lib/sentry/hub.rb:56:in `with_scope'
/app/vendor/bundle/ruby/2.7.0/gems/sentry-ruby-core-4.5.2/lib/sentry-ruby.rb:168:in `with_scope'
/app/vendor/bundle/ruby/2.7.0/gems/sentry-ruby-core-4.5.2/lib/sentry/rack/capture_exceptions.rb:14:in `call'
/app/vendor/bundle/ruby/2.7.0/gems/actionpack-6.1.5/lib/action_dispatch/middleware/executor.rb:14:in `call'
/app/vendor/bundle/ruby/2.7.0/gems/rack-2.2.3/lib/rack/sendfile.rb:110:in `call'
/app/vendor/bundle/ruby/2.7.0/gems/actionpack-6.1.5/lib/action_dispatch/middleware/host_authorization.rb:142:in `call'
/app/lib/same_site_security/middleware.rb:9:in `call'
/app/vendor/bundle/ruby/2.7.0/gems/railties-6.1.5/lib/rails/engine.rb:539:in `call'
/app/vendor/bundle/ruby/2.7.0/gems/railties-6.1.5/lib/rails/railtie.rb:207:in `public_send'
/app/vendor/bundle/ruby/2.7.0/gems/railties-6.1.5/lib/rails/railtie.rb:207:in `method_missing'
/app/vendor/bundle/ruby/2.7.0/gems/puma-5.6.2/lib/puma/configuration.rb:252:in `call'
/app/vendor/bundle/ruby/2.7.0/gems/puma-5.6.2/lib/puma/request.rb:77:in `block in handle_request'
/app/vendor/bundle/ruby/2.7.0/gems/puma-5.6.2/lib/puma/thread_pool.rb:340:in `with_force_shutdown'
/app/vendor/bundle/ruby/2.7.0/gems/puma-5.6.2/lib/puma/request.rb:76:in `handle_request'
/app/vendor/bundle/ruby/2.7.0/gems/puma-5.6.2/lib/puma/server.rb:441:in `process_client'
/app/vendor/bundle/ruby/2.7.0/gems/puma-5.6.2/lib/puma/thread_pool.rb:147:in `block in spawn_thread'
@sengi sengi changed the title Crash when creating an authorisation with an app permission that doesn't exist Crash in /api/v1/api-users/<n>/authorisations when a named permission doesn't exist Mar 28, 2022
@sengi sengi changed the title Crash in /api/v1/api-users/<n>/authorisations when a named permission doesn't exist nil dereference in /api/v1/api-users/<n>/authorisations when a named permission doesn't exist Mar 28, 2022
@sengi sengi changed the title nil dereference in /api/v1/api-users/<n>/authorisations when a named permission doesn't exist nil dereference in api/v1/authorisations#create when a named permission doesn't exist Mar 28, 2022
@floehopper
Copy link
Contributor

I've just validated that this is still an issue by adding a test like this one, but adding a permission name that's not in this list. I saw exactly the same exception, although the stack trace has changed slightly since this issue was reported - the exception is now raised on this line.

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

No branches or pull requests

2 participants