add OAS securitySchemes and security objects

[n2ygk]

Description

adds openapi securitySchemes component and security objects both root level and to operations if needed.

For example:

components:
  ...
  securitySchemes:
    basicAuth:
      type: http
      scheme: basic
      description: Basic Authentication
    sessionAuth:
      type: apiKey
      in: cookie
      name: JSESSIONID
      description: Session authentication

...
paths:
  /v1/courses/:
    get:
      operationId: List/v1/courses/
      description: A course of instruction. e.g. COMSW1002 Computing in Context
      security:
      - basicAuth: []
      - sessionAuth: []

Adds two new classmethods to authentication:

• openapi_security_scheme: returns an OAS securityScheme object
• openapi_security_requirement: returns an OAS security requirements object

The logic behind doing it this way is that anyone that extends BaseAuthentication will be able to add their own OAS securitySchemes and security objects, for example, for OAuth 2.0 in django-oauth-toolkit's TokenMatchesOASRequirements()...

Tests and documentation still need updating but I wanted to get feedback on the general approach before proceeding.

[carltongibson]

Not sure about this. It seems to me that the vast majority of projects set auth once at the project level and don’t modify it per-view, but here we’re adding the extra details with every operation. It seems like overkill.

Ideally we’d add at the schema level and then allow overrides, by using an AutoSchema subclass on the few views with the different authentication classes. In that case helpers to map to the OAS objects would be handy. Maybe we could provide that in a mixin... 🤔

This would need docs and tests.

[n2ygk]

Authentication is top-level in the components.securitySchemes which itemizes which alternative schemes are available (e.g. basic auth, session auth, token auth, etc).

What you describe is putting the authorization security requirement object list at the root level which then applies to the entire schema, unless overridden at the operation level.

If authorizations are in fact the same for all operations, one would need to somehow deduce that for each view iterated over in get_schema() they share a common set of authentication and permission classes.... That would certainly be a good optimization to handle the common project-level case and would enable reducing some duplicated operation-level security objects.

But, in the case where they do differ, by putting the security requirement objects at the operation level, they are consistent with the way CBV's are declared.

Once one gets to permission_classes, especially with OAuth 2.0 scopes (not yet implemented in this PR, but I've done it before and plan on adding that to this PR next -- unless this gets shot down;-), I hope you will see the value of documenting the requirements at the operation level. See the example at https://django-oauth-toolkit.readthedocs.io/en/latest/rest-framework/permissions.html#tokenmatchesoasrequirements reproduced here:

openapi: "3.0.0"
info:
  title: songs
  version: v1
components:
  securitySchemes:
    song_auth:
      type: oauth2
      flows:
        implicit:
          authorizationUrl: http://localhost:8000/o/authorize
          scopes:
            read: read about a song
            create: create a new song
            update: update an existing song
            delete: delete a song
            post: create a new song
            widget: widget scope
            scope2: scope too
            scope3: another scope
paths:
  /songs:
    get:
      security:
        - song_auth: [read]
      responses:
        '200':
          description: A list of songs.
    post:
      security:
        - song_auth: [create]
        - song_auth: [post, widget]
      responses:
        '201':
          description: new song added
    put:
      security:
        - song_auth: [update]
        - song_auth: [put, widget]
      responses:
        '204':
          description: song updated
    delete:
      security:
        - song_auth: [delete]
        - song_auth: [scope2, scope3]
      responses:
        '200':
          description: song deleted

This documents for the client app developer which (alternative sets of) scopes are required by the server for each given path and operation.

Basically for the security object, the name is the type of authentication and the list is the authorizations. When authorization is done strictly inside the app, the list is empty, but some level of high-level authorization can be indicated in the OAS schema as well.

Here's a snippet example for declaring the OAuth2 securityScheme component. I've used this stuff with Swagger-UI "Try It" and it actually works;-)

class MyOAuth2Auth(OAuth2Authentication):
    openapi_security_scheme_name = 'oauth2ForYou'

    @classmethod
    def openapi_security_scheme(cls):
        scheme = {
            cls.openapi_security_scheme_name: {
                'type': 'oauth2',
                'description': 'OAuth 2.0 authentication'
            }
        }
        flows = {}
        if 'authorization_code' in settings.OAUTH2_CONFIG['grant_types_supported']:
            flows['authorizationCode'] = {
                'authorizationUrl': settings.OAUTH2_CONFIG['authorization_endpoint'],
                'tokenUrl': settings.OAUTH2_CONFIG['token_endpoint'],
                'refreshUrl': settings.OAUTH2_CONFIG['token_endpoint'],
                'scopes': {s: s for s in settings.OAUTH2_CONFIG['scopes_supported']}}
        if 'implicit' in settings.OAUTH2_CONFIG['grant_types_supported']:
            flows['implicit'] = {
                'authorizationUrl': settings.OAUTH2_CONFIG['authorization_endpoint'],
                'scopes': {s: s for s in settings.OAUTH2_CONFIG['scopes_supported']}}
        if 'client_credentials' in settings.OAUTH2_CONFIG['grant_types_supported']:
            flows['clientCredentials'] = {'tokenUrl': settings.OAUTH2_CONFIG['token_endpoint'],
                'refreshUrl': settings.OAUTH2_CONFIG['token_endpoint'],
                'scopes': {s: s for s in settings.OAUTH2_CONFIG['scopes_supported']}}
        if 'password' in settings.OAUTH2_CONFIG['grant_types_supported']:
            flows['password'] = {'tokenUrl': settings.OAUTH2_CONFIG['token_endpoint'],
                'refreshUrl': settings.OAUTH2_CONFIG['token_endpoint'],
                'scopes': {s: s for s in settings.OAUTH2_CONFIG['scopes_supported']}}
        scheme[cls.openapi_security_scheme_name]['flows'] = flows

This generates this OAS schema fragment:

  securitySchemes:
    oauth2ForYou:
      type: oauth2
      description: OAuth 2.0 authentication
      flows:
        authorizationCode:
          authorizationUrl: https://oauth-test.cc.columbia.edu/as/authorization.oauth2
          tokenUrl: https://oauth-test.cc.columbia.edu/as/token.oauth2
          refreshUrl: https://oauth-test.cc.columbia.edu/as/token.oauth2
          scopes:
            address: address
            read: read
            openid: openid
            profile: profile
            update: update
            auth-columbia: auth-columbia
            delete: delete
            auth-none: auth-none
            offline_access: offline_access
            https://api.columbia.edu/scope/group: https://api.columbia.edu/scope/group
            create: create
            demo-djt-sla-bronze: demo-djt-sla-bronze
            email: email
            cas-tsc-sla-gold: cas-tsc-sla-gold
        implicit:
          authorizationUrl: https://oauth-test.cc.columbia.edu/as/authorization.oauth2
          scopes:
            address: address
            read: read
            openid: openid
       ...

I still need to get to the openapi_security_requirement() definition which needs to inspect the CBV's permission_classes. I have code that does it, but first wanted to get feedback on whether you think this is a good approach to automating making a fully functional OAS document that Swagger UI can use.

[carltongibson]

Okay, good.

So I guess here's my question, does it make sense to skip adding the security details when authentication_classes == api_settings.DEFAULT_AUTHENTICATION_CLASSES? (I.e. they've not been overridden?)

[carltongibson]

@n2ygk Just to clarify: I think this is a good addition. Please do continue. 🙂

[n2ygk]

Okay, good.

So I guess here's my question, does it make sense to skip adding the security details when authentication_classes == api_settings.DEFAULT_AUTHENTICATION_CLASSES? (I.e. they've not been overridden?)

That's a great idea. I'll also use the defaults, if set, to create a root-level security requirements object.

[n2ygk]

@carltongibson I've incorporated your suggestions, cleaned up the tests and added new ones as well as documentation, and I've rebased. I look forward to your feedback. Cheers!

[n2ygk]

Oops this last rebase broke due to url vs. path. I should have run tox locally first. Fixing it now.

[n2ygk]

hmm, @carltongibson your review comment seems to have disappeared. Maybe github is being eventually consistent?

[carltongibson]

OK, this looks good. With more time I'd like to go over and regroup some of the newer additions, but that's not a criticism of this.

This should be a nice addition. We can come back and tidy later. Thanks @n2ygk

[carltongibson]

@n2ygk — No I deleted it, it wasn't right. The code there is getting a little procedural — that's OK, we can look at it over time — and I'd misread my place. First pass, i think this does the job and we should probably have it.

I asked Tom to review as I didn't have time to make smaller comments. Maybe he will, maybe he won't.

[n2ygk]

@tomchristie I don't mean to be annoying but am wondering what the likelihood is of this PR being accepted. I see this milestone is due in a few days.

[tomchristie]

Okay, just got through to this one in my notifications. As it happens I've just rolled a point release. Because.
But I'm okay with working through the 3.13 milestoned issues likely with a view to a 3.13 release in about a months time.

[n2ygk]

Okay, just got through to this one in my notifications. As it happens I've just rolled a point release. Because.
But I'm okay with working through the 3.13 milestoned issues likely with a view to a 3.13 release in about a months time.

@tomchristie FYI - I've rebased in hopeful anticipation of your imminent review ;-)

[n2ygk]

@tomchristie @carltongibson I see you've merged a new 3.13 release. Is this PR still on the queue for eventual review? Thanks.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[n2ygk]

Not stale! See #8453

[carltongibson]

TBH I would rather see this sort of hook added via a decorator (or such) rather than added in as part of the DRF API.

[n2ygk]

@carltongibson Could you provide an example of how that might work? I don't quite understand.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[auvipy]

@tfranzel can we or do we need this feature ported to drf-spectacular?

[tfranzel]

@auvipy yes I think so. Since schema information on auth was completely missing in DRF (unlike for ex. pagination), we created extensions that do exactly what was added here.

https://github.com/tfranzel/drf-spectacular/blob/ceff2fc9c6deb509b300bbf82ee8d88d00cd8eb6/drf_spectacular/extensions.py#L13

Based on that, we implemented extensions for all build-in DRF auth methods, as well as the most popular 3rd party auth apps:

https://github.com/tfranzel/drf-spectacular/blob/master/drf_spectacular/authentication.py
https://github.com/tfranzel/drf-spectacular/tree/master/drf_spectacular/contrib

We can make use of this added information in case there is no extensions. We would need to keep the existing extensions due to compatibility issues with older DRF versions anyway. I currently don't really have an opinion on how exactly this information should be added.

So from my side there are no blockers, however this thing has gotchas. We ended up extending the interface twice to cover missed use-cases, e.g. multiple headers for one auth. Just thought this is worth mentioning.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.