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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should AzureAD Tests Pass? #60

Open
plamenGo opened this issue Apr 22, 2020 · 7 comments
Open

Should AzureAD Tests Pass? #60

plamenGo opened this issue Apr 22, 2020 · 7 comments

Comments

@plamenGo
Copy link

Hello!

Have you seen this test suite for SCIM endpoints? I ran and got a fair amount of failing tests ~50%

https://github.com/AzureAD/SCIMReferenceCode/wiki/Test-Your-SCIM-Endpoint

Digging into the failures some of them do not seem very significant, and changes in route/ response structure, etc make the tests pass. Not sure how significant this is, and whether it might impede actual Azure AD integration.

What SCIM providers have you tested this, or know of this running successfully with. GSuite, Okta?

Thank you so much!
Plamen

@plamenGo plamenGo changed the title Should AzureAD Tests Pass Should AzureAD Tests Pass? Apr 22, 2020
@imulab
Copy link
Owner

imulab commented Apr 24, 2020

Thanks for letting me know about this test suite. Let me take a look!

@plamenGo
Copy link
Author

@imulab if you want me to help, I can take a stab at fixing some of these test failures. Just let me know which ones you consider important, and/or which ones I should look at.

@plamenGo
Copy link
Author

It looks like, at least partially, the issue is that the Azure AD Postman test suite expects the enterprise extension schema to be supported. This user failing to get created triggers other failures downstream.

The failing test in question is: Post EnterpriseUser. This body is sent to POST /Users:
{ "UserName": "UserName222", "Active": true, "DisplayName": "lennay", "schemas": [ "urn:ietf:params:scim:schemas:extension:enterprise:2.0:User", "urn:ietf:params:scim:schemas:core:2.0:User" ], "externalId": "${__UUID}", "name": { "formatted": "Adrew Ryan", "familyName": "Ryan", "givenName": "Andrew" }, "emails": [ { "Primary": true, "type": "work", "value": "testing@bob2.com" }, { "Primary": false, "type": "home", "value": "testinghome@bob3.com" } ], "urn:ietf:params:scim:schemas:extension:enterprise:2.0:User": { "Department": "bob", "Manager" : { "Value": "SuzzyQ" } } }
Resulting in this error code:

{ "schemas": [ "urn:ietf:params:scim:api:messages:2.0:Error" ], "status": 400, "scimType": "invalidPath", "detail": "invalidPath: no attribute named 'urn:ietf:params:scim:schemas:extension:enterprise:2.0:User' from ''" }

@imulab
Copy link
Owner

imulab commented Apr 30, 2020

Thanks for taking the initiative! I downloaded the test suite and had a brief look, but was a bit too busy to go through it thoroughly.

The core module is designed to support any extensions, which surely includes urn:ietf:params:scim:schemas:extension:enterprise:2.0:User.

The reason you are seeing this error is because the top level server module is designed to be an opinionated showcase (i.e. note, the purpose of this project is to provide building blocks, see the project description). It wasn't shipped with the enterprise extension schema, and the User resource type also didn't define that extension either. In this case, server validation would not know urn:ietf:params:scim:schemas:extension:enterprise:2.0:User field when it sees it.

If you want to setup the enterprise extension, the following needs to be done:

  1. Put a enterprise_user_extension_schema.json into the schemas directory (name doesn't matter that much, but the id URN does!). The schema file needs to follow the format of other schema files to correctly specify the SCIM defined attributes as well as the extension attributes supported by this project.
  2. In user_resource_type.json, add schemaExtensions field and specify the URN of the enterprise user extension schema.
  3. If necessary, include a MongoDB metadata definition for the added schema. This is occasionally necessary because SCIM fields (i.e. $ref) collides with MongoDB field names, and will need to be mapped when stored. There is already one in the public/mongo_metadata folder.

Then, when you start the server with the correct options pointing to those resource locations, the schemas would be parsed first and cached in memory, then the resource type will be parsed to know that the User resource would have enterprise extensions.

I am open to discussion whether you think it is beneficial to include the enterprise extension by default in the top level showcase project, as I see a lot of people use this project to support their AzureAD use cases. I surely would like to learn more about it.

@plamenGo
Copy link
Author

plamenGo commented Apr 30, 2020

Yes, we should definitely work to pass the Microsoft SCIM reference code tests with this project. This would ensure compliance and support for Azure AD. I opened a PR for a small test fix, and will continue looking at these Postman tests to try and get them to pass. I added an enterprise user schema, but I need to verify that it defines all the attributes -- some of the tests still fail -- the server throws errors similar to these:
`
{"level":"error","error":"invalidSyntax: expects null (pos:14)","time":"2020-04-30T19:15:54Z","message":"error when patching resource"}

{"level":"error","error":"invalidValue, value for 'meta.created' does not conform to ISO8601","time":"2020-04-30T19:15:54Z","message":"error when creating resource"}

{"level":"error","error":"invalidPath: no attribute named 'displayName' from 'members'","time":"2020-04-30T19:15:53Z","message":"error when patching resource"}

{"level":"error","error":"invalidPath: error compiling path","time":"2020-04-30T19:15:52Z","message":"error when searching resource"}
`
I don't know enough about the implementation to know exactly how to fix these, but working on it``

@imulab
Copy link
Owner

imulab commented May 1, 2020

I think I agree with you. Let me work up a branch that sets up the enterprise extension first.

As for the above errors:

  • I would need more context for the first one and the last one.
  • The second one seems to be complaining about the ISO8601 format as it expects that field to be a ISO8601 formatted time string. What does meta.created Contain?
  • For the third, it seems to be complaining about the members.displayName Attribute. Reading from the core spec section 8.4, the group representation has the attribute as members.display, not members.displayName.

I don’t have my computer with me now, so just laying down some guess work on an iPad. Will take another look when I am back.

Meanwhile, could you provide more context on the above failed tests? I don’t mind if you want open up bug reports for those. Thanks!

@plamenGo
Copy link
Author

plamenGo commented May 1, 2020

{"level":"error","error":"invalidPath: no attribute named 'displayName' from 'members'","time":"2020-04-30T19:15:53Z","message":"error when patching resource"}

This specific test failure seems to be a problem with the test. Opened an issue for it on the Microsoft repo: AzureAD/SCIMReferenceCode#20

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants