strongloop / loopback-component-oauth2

oAuth 2.0 server for LoopBack
http://www.strongloop.com
Other
62 stars 63 forks source link

Further integration between OAuth2 and LoopBack #55

Closed JonnyBGod closed 7 years ago

JonnyBGod commented 7 years ago

Description

WIP

Better integrate oauth2 component with loopback.

  1. Unify the access token model between oAuth 2.0 and User.login
  2. Integrate scope with ACL

Related issues

https://github.com/strongloop/loopback/issues/817

Checklist

Missing:

slnode commented 7 years ago

Can one of the admins verify this patch? To accept patch and trigger a build add comment ".ok\W+to\W+test."

slnode commented 7 years ago

Can one of the admins verify this patch?

slnode commented 7 years ago

Can one of the admins verify this patch?

slnode commented 7 years ago

Can one of the admins verify this patch?

raymondfeng commented 7 years ago

@JonnyBGod Thank you for the patch. I'll find some time to review it.

JonnyBGod commented 7 years ago

@raymondfeng This is not finished as I still need to implement tests for it. But before that I would appreciate an overall review on my approach.

drmikecrowe commented 7 years ago

@JonnyBGod How exactly does this integrate within the Loopback ACL system?

I have a fork I'm working on of loopback-swagger that adds the securityDefinitions and security sections to swagger.json. Now, I'm searching for how to add the OAuth2 scopes to a method (built-in or remote). The global scope works fine.

Example: In components-config.json:

    "securityDefinitions": {
        "OauthSecurity": {
            "type": "oauth2",
            "flow": "accessCode",
            "name": "access_token",
            "authorizationUrl": "http://localhost:3006/oauth/authorization",
            "tokenUrl": "http://localhost:3006/oauth/token",
            "scopes": {
                "read": "Read Only Permission",
                "write": "Read/Write Permission"
            }
        }
    },
    "security": [
        {
            "OauthSecurity": [
                "read"
            ]
        }
    ]

However, in a given method, we need (as an example) the method to specify the scope restriction such the following:

  "/Devices/{id}/removeDevice": {
    "post": {
      "tags": [
        "Device"
      ],
      "operationId": "Device.removeDevice",
      "parameters": [
        {
          "name": "id",
          "in": "path",
          "description": "Device ID",
          "required": true,
          "type": "string"
        }
      ],
      "responses": {
        "200": {
          "description": "Request was successful",
          "schema": {
            "description": "Returns the deleted Device if successful",
            "$ref": "#/definitions/Device"
          }
        }
      },
      "deprecated": false,
      "security": [
        {
          "OauthSecurity": [
            "write"
          ]
        }
      ]
    }
  }
JonnyBGod commented 7 years ago

I just mean that resolving ACLs is working fine with this patch. Have not actually tested scopes. Might need more work for that to work.

drmikecrowe commented 7 years ago

@raymondfeng What was the long-term plan for how to integrate OAuth2 scopes into ACL's?

Would integrating them into ACL's using a different role be acceptable?

  "acls": [
    {
      "accessType": "EXECUTE",
      "principalType": "OAUTH2",
      "principalId": "removeDevice",
      "scope": "OauthSecurity:write",
      "permission": "ALLOW"
    }
  ]

This will definitely require a customer role-resolver in this component.

drmikecrowe commented 7 years ago

NOTE: I'm working on a loopback-swagger fork which impacts this: It's a work-in-progress that supports the configuration of OAuth2 in the swagger.json. Still needs automated tests, but the results work perfectly in http://editor.swagger.io/#!/

https://github.com/drmikecrowe/loopback-swagger

raymondfeng commented 7 years ago

@drmikecrowe This PR will get us close to oAuth2 alignment - https://github.com/strongloop/loopback/pull/3313.

stale[bot] commented 7 years ago

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[bot] commented 7 years ago

This issue has been closed due to continued inactivity. Thank you for your understanding. If you believe this to be in error, please contact one of the code owners, listed in the CODEOWNERS file at the top-level of this repository.