Skip to content

Automatic checks of authentication / authorization / instance publicity

Changes the approach to security / privacy checks from [mostly] disabled by default (unless explicitly [and properly] added by each committer) to enabled by default (unless explicitly and legitimately disabled by developers for a minor number of actions).

  • In controllers, use Pleroma.Web, :controller will result in action/2 be called prior to actual controller action, and it'll perform security / privacy checks before passing control to actual controller action.

    For routes with :authenticated_api pipeline, authentication & authorization are expected, thus OAuthScopesPlug will be run unless explicitly skipped (also EnsureAuthenticatedPlug will be executed immediately before action even if there was an early run to give an early error, since OAuthScopesPlug supports fallback: :proceed_unauthenticated option which will drop auth info if scopes check failed [pipeline-level call to EnsureAuthenticatedPlug would already be passed at this point], and other plugs may support similar options as well).

    For :api pipeline routes, it'll be verified whether OAuthScopesPlug was called or explicitly skipped, and if it was not then auth information will be dropped for request. Then EnsurePublicOrAuthenticatedPlug will be called to ensure that either the instance is not private or user is authenticated (unless explicitly skipped). Such automated checks help to prevent human errors and result in higher security / privacy for users.

  • As a result, removed a whole lot of manual plug(:EnsurePublicOrAuthenticatedPlug) from controllers, decoupled EnsurePublicOrAuthenticatedPlug from OAuthScopesPlug, removed :skip_instance_privacy_check option for OAuthScopesPlug.

  • Defined OAuth permissions for a whole lot of AdminAPIController actions — surprisingly, scope definitions were partially missing (30 failed tests) even in this controller which has higher security risks then regular API controllers (back in a day there was a commit where scopes were defined for all of AdminAPIController actions — apparently, some committers just added actions and did not define OAuth scopes for them, despite existing definitions for existing actions — I don't think there's a more solid proof that we need OAuthScopesPlug run by default and skipped on demand instead of vice versa).

  • Defined / adjusted OAuth permissions for :api and :authenticated_api pipelines' controllers.

  • Renamed controller actions which were ambiguosly named and raised questions on assigned OAuth scopes (e.g. read_notifications -> mark_notifications_as_read which clarifies assigned write:notifications scope; or follows [POST action] -> follow_by_uri; or download_from [POST] to save_from — that distinguishes it from GET download_shared).

  • Added docs/dev.md to document notes / implementation specifics for other developers (it'd be good if we all could later on document specifics of the system in such doc).

Note on changes in router.ex: !2409 (diffs, comment 56462)

No routes were deleted (even surely incorrect POST /api/pleroma/emoji/packs/list_from is still there but now with a proper GET sibling); most changes in this file (besides :expect_authentication / :expect_public_instance_or_authentication pipelines) are sorting ones (was checking every action for OAuth scopes and grouped actions by controller, so the new ordering).

Notable changes:

Moved from :api to :authenticated_api:

  • get("/apps/verify_credentials", AppController, :verify_credentials) (obviously needs OAuth token to be executed)
  • get("/timelines/list/:list_id", TimelineController, :list) (has existing OAuth scope check; also demands list id, and reading lists to get their ids require "read:lists" scope anyways)

Made auth-optional (as per :api pipeline):

  • get("/accounts/:id/favourites", AccountController, :favourites) (controller-level OAuthScopesPlug call change)
Edited by Ivan Tashkinov

Merge request reports