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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

aws_cognito_resource_server API does not match the AWS API #15443

Open
pll opened this issue Oct 2, 2020 · 4 comments
Open

aws_cognito_resource_server API does not match the AWS API #15443

pll opened this issue Oct 2, 2020 · 4 comments
Labels
enhancement Requests to existing resources that expand the functionality or scope. stale Old or inactive issues managed by automation, if no further action taken these will get closed.

Comments

@pll
Copy link

pll commented Oct 2, 2020

Community Note

  • Please vote on this issue by adding a 馃憤 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

Affected Resource(s)

  • aws_cognito_resource_server

Description

The current documentation for the resource aws_cognito_resource_server specifies that custom scopes should be defined using scope {} blocks, which are maps with the keys scope_name and scope_description.

However, both the Boto3 and the AWS API specify that scopes should be defined as a list of maps:

Scopes (list) -- A list of scopes. Each scope is map, where the keys are name and description .

The problem with the current implementation in Terraform is that, when assigning custom scopes to a aws_cognito_user_pool_client resource, the allowed_oauth_scopes takes a list of strings. Which means, if I know the order of the list the scopes were added to, I could simply index into the list.

Unfortunately, the current implementation does not indicate any order to the resulting list aws_cognito_resource_server.foo.scope_identifiers, which makes indexing into the list unreliable. At present, the best alternative I have is to concoct the name of the specific scope I wish to add to my app client by using something like this:

allowed_oauth_scopes = [ "${aws_cognito_resource_server.foo.identifier}/SCOPE_NAME" ]

This is not ideal, since, if I need to change the name of the scope, I now have to do so in multiple places.

I think a better solution would be to define custom scopes according to the AWS API like this:

resource "aws_cognito_resource_server" "tms" {
    user_pool_id = aws_cognito_user_pool.foo.id
    name = "My Cognito Server"
    identifier = aws_cognito_user_pool_domain.foo.domain

    scope {
        [
          {
            scope_name = "SCOPE_ONE"
            scope_description = "First Scope"
          },
          {
            scope_name = "SCOPE_TWO"
            scope_description = "Second Scope"
          }
       ]
    }
}

Presumably, this would then result in aws_cognito_resource_server.foo.scope_identifiers being a list of all scopes configured for this resource server in the format identifier/scope_name, and in the order defined by the scope list declaration.

References

Thanks,
Paul

@pll pll added the enhancement Requests to existing resources that expand the functionality or scope. label Oct 2, 2020
@ghost ghost added the service/cognito label Oct 2, 2020
@github-actions github-actions bot added the needs-triage Waiting for first response or review from a maintainer. label Oct 2, 2020
@bflad
Copy link
Member

bflad commented Oct 2, 2020

Hi @pll 馃憢 Thank you for raising this. At first look, I was a little confused by this report because the aws_cognito_resource_server resource schema does indeed match the underlying API implementation in the semantics that Terraform understands. Essentially what the Terraform ecosystem considers as "configuration blocks" are an ordered or unordered list of maps already. Today the scopes schema is modeled as an "unordered set of maps", which sounds like it might be problematic in this specific use case.

To further understand how to help you, it would be helpful to understand the final configuration you are looking to achieve and how it does not work correctly today. Can you please show more details? Thanks.


Aside: The Terraform configuration language specification cannot support the proposed configuration since blocks can only contain identifiers (arguments). An alternative implementation of the following may be possible (directly a list of maps), however this is not a pattern that is not well used in the Terraform ecosystem and it would require a lot of additional resource logic to attempt implementing validations, etc. since this is outside the standard definition of an open-ended key-value map:

# Example configuration directly using a list of maps.
# This example is not a pattern recommended in Terraform Providers given
# schema handling complexities and untested behaviors in the Terraform
# plan difference handling. Instead, configuration blocks are recommended.
resource "aws_cognito_resource_server" "example" {
  # ... other configuration ...

  scope = [
    {
      scope_name = "SCOPE_ONE"
      scope_description = "First Scope"
    },
    {
      scope_name = "SCOPE_TWO"
      scope_description = "Second Scope"
    }
  ]
}

@bflad bflad added waiting-response Maintainers are waiting on response from community or contributor. and removed needs-triage Waiting for first response or review from a maintainer. labels Oct 2, 2020
@pll
Copy link
Author

pll commented Oct 2, 2020

Hi @bflad,

The problem is, it's an unordered list, which to me, does not guarantee any kind of order. Therefore, let's take the following scenario:

  • Create a resource server with 2 scope{} blocks, we'll call them scope1 and scope2.
  • Create 2 separate and distinct aws_user_pool_clients, we'll call them clientA and clientB.
  • I want clientA to get scope1 and clientBto getscope2`.

Currently, I have no reliable means of doing that short of explicitly assigning each scope by name like this:

allowed_oauth_scopes = [ "${aws_cognito_resource_server.foo.identifier}/scope1" ]

If you look at how the state file get set, it looks like this:

  "module": "module.foo",
  "mode": "managed",
  "type": "aws_cognito_resource_server",
  "name": "foo",
  "provider": "provider.aws",
  "instances": [
    {
      "schema_version": 0,
      "attributes": {
        "id": "us-east-1_gtglmFgt6|pll-dev",
        "identifier": "pll-dev",
        "name": "auth-plldev",
        "scope": [
          {
            "scope_description": "Scope 1",     <--- Defined first
            "scope_name": "scope1" 
          },
          {
            "scope_description": "Scope 2",    <--- Defined second
            "scope_name": "scope2"
          }
        ],
        "scope_identifiers": [
          "pll-dev/scope2",       <--- index 0
          "pll-dev/scope1".        <--- index 1
        ],

So, despite the fact that I defined scope1 first in the block, because they're unordered, they don't end up in the array where I expect them. Therefore, I can't do something like:

allowed_oauth_scopes = [ aws_cognito_resource_server.foo.scope_identifiers[0] ]

and reliably expect to get pll-dev/scope1

I'm forced to do this:

allowed_oauth_scopes = [ "${aws_cognito_resource_server.foo.identifier}/scope1" ]

Make sense ?

Thanks.
Paul

@ghost ghost removed the waiting-response Maintainers are waiting on response from community or contributor. label Oct 2, 2020
@crablab
Copy link

crablab commented Jul 19, 2021

馃憢 Just wanted to follow up on this as we've been having issues with this as well.

As @pll says, the problem is scope_identifiers has no deterministic order as a list, and therefore it's difficult to refer to the scopes within it. We have a slightly more creative solution with regex to find scopes within the list, but it's not ideal as our code isn't DRY.

I wonder if a map indexed by scope name would be a more sensible way of representing this? That would still allow iteration, but would also allow you to refer to a scope thus: aws_cognito_resource_server.foo.scope_identifiers.scope1.

Copy link

Marking this issue as stale due to inactivity. This helps our maintainers find and focus on the active issues. If this issue receives no comments in the next 30 days it will automatically be closed. Maintainers can also remove the stale label.

If this issue was automatically closed and you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thank you!

@github-actions github-actions bot added the stale Old or inactive issues managed by automation, if no further action taken these will get closed. label Apr 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Requests to existing resources that expand the functionality or scope. stale Old or inactive issues managed by automation, if no further action taken these will get closed.
Projects
None yet
Development

No branches or pull requests

4 participants