1. 05 Jul, 2017 1 commit
    • Timothy Andrew's avatar
      Squashed commit representing changes in gitlab-ce!12300. · 6d0dad64
      Timothy Andrew authored
      - There were conflicting changes in `master` that were fixed in
        94258a65. This made rebasing the commits from gitlab-ce!12300
        problematic, due to conflicts.
      
      - Instead, I squashed all !12300 commits into a single commit, and
        cherry-picked that onto 33580-fix-api-scoping-ee, which resulted
        in this commit.
      
      Original commit messages below
      ==============================
      
      Initial attempt at refactoring API scope declarations.
      
      - Declaring an endpoint's scopes in a `before` block has proved to be
        unreliable. For example, if we're accessing the `API::Users` endpoint - code
        in a `before` block in `API::API` wouldn't be able to see the scopes set in
        `API::Users` since the `API::API` `before` block runs first.
      
      - This commit moves these declarations to the class level, since they don't need
        to change once set.
      
      Allow API scope declarations to be applied conditionally.
      
      - Scope declarations of the form:
      
          allow_access_with_scope :read_user, if: -> (request) { request.get? }
      
        will only apply for `GET` requests
      
      - Add a negative test to a `POST` endpoint in the `users` API to test this. Also
        test for this case in the `AccessTokenValidationService` unit tests.
      
      Test `/users` endpoints for the `read_user` scope.
      
      - Test `GET` endpoints to check that the scope is allowed.
      - Test `POST` endpoints to check that the scope is disallowed.
      - Test both `v3` and `v4` endpoints.
      
      When verifying scopes, manually include scopes from `API::API`.
      
      - They are not included automatically since `API::Users` does not inherit from
        `API::API`, as I initially assumed.
      
      - Scopes declared in `API::API` are considered global (to the API), and need to
        be included in all cases.
      
      Test OAuth token scope verification in the `API::Users` endpoint
      
      Add CHANGELOG entry for CE MR 12300
      
      Fix remaining spec failures for !12300.
      
      1. Get the spec for `lib/gitlab/auth.rb` passing.
      
        - Make the `request` argument to `AccessTokenValidationService` optional -
        `auth.rb` doesn't need to pass in a request.
      
        - Pass in scopes in the format `[{ name: 'api' }]` rather than `['api']`, which
        is what `AccessTokenValidationService` now expects.
      
      2. Get the spec for `API::V3::Users` passing
      
      2. Get the spec for `AccessTokenValidationService` passing
      
      Implement review comments from @dbalexandre for !12300.
      
      Implement review comments from @DouweM for !12300.
      
      - Use a struct for scopes, so we can call `scope.if` instead of `scope[:if]`
      
      - Refactor the "remove scopes whose :if condition returns false" logic to use a
        `select` rather than a `reject`.
      
      Extract a `Gitlab::Scope` class.
      
      - To represent an authorization scope, such as `api` or `read_user`
      - This is a better abstraction than the hash we were previously using.
      
      `AccessTokenValidationService` accepts `String` or `API::Scope` scopes.
      
      - There's no need to use `API::Scope` for scopes that don't have `if`
        conditions, such as in `lib/gitlab/auth.rb`.
      
      Fix build for !12300.
      
      - The `/users` and `/users/:id` APIs are now accessible without
        authentication (!12445), and so scopes are not relevant for these endpoints.
      
      - Previously, we were testing our scope declaration against these two methods.
        This commit moves these tests to other `GET` user endpoints which still
        require authentication.
      6d0dad64
  2. 04 Jul, 2017 26 commits
  3. 03 Jul, 2017 13 commits