[DRAFT] Add descriptions and some formats to the Profile schema by djmitche · Pull Request #72 · mozilla-iam/cis

djmitche · 2017-08-03T22:19:54Z

I've written up some descriptions for schema fields -- and added a few formats where they were clear.

This is all just guesswork -- and where I've written "UNKNOWN" I was unable to guess. Please comment to let me know where I've gotten it wrong, and I will update.

djmitche · 2017-08-07T17:09:13Z

It looks like this has been moved to cis/plugins/validation/schema.json. I don't know why git couldn't follow the move. I'll rebase once I've gotten some comments. @gdestuynder - any comments?

djmitche · 2017-08-11T13:49:33Z

@gdestuynder

djmitche · 2017-08-11T13:50:48Z

damn 🐱

gdestuynder · 2017-08-14T09:45:00Z

Thanks for this PR, this is quite useful

I've edited it to reflect how fields are used in some cases (or fixed the UNKNOWNs)
I've also removed the wording related to 'authoritative' vs 'informational' as this would not currently reflect how the profile is generated.
All values come for various user management interfaces and IdP (workday, mozillians.org, ldap mainly). We more generally call these CIS providers in the code and they may only touch certain attributes, based on validation rules that we maintain in code (they are not expected to change often).

Finally, the file has moved to https://github.com/mozilla-iam/cis/blob/master/cis/plugins/validation/schema.json hence the conflict

gdestuynder · 2017-08-14T09:53:21Z

manually merging with conflict resolution

djmitche

Thanks for landing this -- it's a great update. I've made some more comments here, and I will make another PR based on the answers.

djmitche · 2017-08-14T14:20:08Z

cis/schema.json

        "user_id": {
            "type": "string",
-            "description": "(Authoritative) The user ID as defined by the underlying IdP. Profile consumers should not try to interpret this value in any way, but may use it as a stable identifier for this user as identified by this IDP."
+            "description": "The unique user identifier. Profile consumers should not try to interpret this value in any way."


Is it not allowed to use this as a stable identifier? What should a client use to identify "the same" user from session to session?

i don't want readers to infer that this is to be used as the user identifier for users to login, i.e.
Login: blah|LDAP|xxx
Password: xxxx

Because the user_id is not very human friendly compared to emails. It's ok to use it internally where needed though.

djmitche · 2017-08-14T14:21:37Z

cis/schema.json

        "active": {
            "type": "boolean",
-            "description": "(Authoritative) If false, all access for this user should be denied. Profile consumers MUST check this value and disregard the remainder of the profile if false."
+            "description": "If false, all access for this user should be denied. Profile consumers MUST check this value and disregard the remainder of the profile if false."


Is there a circumstance where this would be false but the profile still returned?

when querying the CIS API in the future - the point is that it is stored that way in the identity vault (ie the database of user profiles) thus anything that may read the profile would get "false" returned. Right now, this is only "internal" CIS code. Auth0 will return an HTTP error code.

djmitche · 2017-08-14T14:22:03Z

cis/schema.json

        "userName": {
            "type": "string",
-            "description": "UNKNOWN"
+            "description": "Human-readable login name or identifier for this user. For a unique identifier, use `user_id` instead."


Hm, but user_id doesn't say it's stable..

See above comment about user_id

djmitche · 2017-08-14T14:25:13Z

cis/schema.json

        },
        "PGPFingerprints": {
            "type": "array",
+            "description": "All PGP fingerprints associated with this user's profile such as `bob` or `mary`. Order is undefined.",


Are bob and mary fingerprints?

:) if you pr, please fix

djmitche · 2017-08-14T14:26:37Z

cis/schema.json

        "groups": {
            "type": "array",
-            "description": "(Informational) Deprecated. Use `authoritativeGroups` instead.",
+            "description": "Informational group membership for the user. Non-prefixed groups are what is known as Mozilla LDAP groups. Groups prefixed by a provider name and underscore are provided by a specific group engine.\nFor example `providername_groupone` is provided by `providername`.\nThese groups are provided `as-is` and you should ensure it contains the members you expect.",


Perhaps add a reference to authoritativeGroups here and say in as many words that groups is not suitable for access control?

groups are used for access control. how much of a guarantee that groups are properly managed is up to you (it's managed by humans, which are generally also the owner of the service using these groups, but not necessarily), and authoritativeGroups have a better guarantee because they are either well defined with regular humans audits, either programatically defined. Both are needed in practice.

djmitche · 2017-08-14T14:28:25Z

cis/schema.json

        "authoritativeGroups": {
            "type": "array",
-            "description": "(Authoritative) A list of user-groups of which this user is a member. The group names are specific to the IdP, so profile consumers should take the IdP into account when interpreting this information. Note that the identity of the IdP is not available in the profile.",
+            "description": "A highy reliable list of groups for which this user is a member. These groups are either machine generated and strongly verified to be correct through software, or follow a well-defined human process in order to be verified to be correct.\nThese groups may optionally be used by the access providers themselves to assert access and may take precedence over the RP's own group checks.",


djmitche · 2017-08-14T15:32:46Z

cis/schema.json

                        "type": "string",
                        "format": "date-time",
-                        "description": "UNKNOWN"
+                        "description": "Date at which this group membership last granted access to a resource (generally, an RP) for the user. This may be used to automatically expire and deny access when the lastUsed attribute is older than 3 month, for example."


Is there a defined way to update this date? Or is that something that's only handled for very specific cases (I'm assuming VCS access..).

So far we've only planned to update this date through CIS, by Auth0, on-access. VCS still uses an LDAP write to a specific LDAP attribute at this time. There could be other publishers such as a VCS in the future though, effectively replacing the LDAP attribute write.

djmitche · 2017-08-14T15:38:29Z

cis/schema.json

                    "name": {
                        "type": "string",
-                        "description": "Name of the group, as given by the IdP."
+                        "description": "Name of the group."


Does this follow the same format (provider_name)?

No - while this structure (authoritativeGroups) is present it's not yet used - but we currently do not plan to namespace publishers within it. We had a previous iteration where all groups were namespaced (via a json structure, not a string 'hack'/underscore) but we realized it was currently too unrealistic. This is also why, at this time, only Auth0 is planned to write here.

Interesting. If the set of authoritativeGroups is small enough, perhaps each known group could be enumerated in the schema, with suitable descriptions?

djmitche · 2017-08-14T15:46:28Z

cis/schema.json

                    "uuid": {
                        "type": "string",
-                        "description": "Reserved; profile providers should not use this value."
+                        "description": "Unique identifier for the authoritative group. Profile consumers should not try to interpret this value in any way."


If I were building a document editing tool and wanted to be able to associate groups with access to a document, would this be the right value to store in my access-control table, rather than name?

if you wanted to use these groups, then yes. In most cases you'll want to use groups though.

There were many questions about groups vs authoritativeGroups and ideally I/we should make a little doc about it (descriptions are a little short to tell the whole story).

Another attempt through comments though:

Auth0 (the access provider) is expected to maintain and enforce AuthoritativeGroups. This means it may prevent a user to login to an RP, regardless of the RP's opinion on this. While all RPs have an associated AuthoritativeGroups attribute, not all of the RPs access is prevented by Auth0 (basically for any RP set to "allow anyone in" Auth0 does not prevent access, it just updates attributes, otherwise, it does/will)

RPs may use AuthoritativeGroups values if they wish, though Auth0 already did the access validation on their behalf

RPs should use groups (any, or combination of groups, as the RP sees fit) with full knowledge that the groups are only as reliable as the people managing them. eg:

if there is a group called "mozilliansorg_pplwholikechocolate" managed by a person in Japan that maintain a directory of friends who like chococolate and you decide to use it, it's only as reliable as what this person decides the members are.

If you create your own "mozilliansorg_pplthatitrust" (mozillians.org) or "pplthatitrust" (LDAP manage) then it's up to you.

If you use "workday_staff", these are managed by Mozilla HR, though they currently do not have the same guarantees as AuthoritativeGroups, but they hopefully and eventually will. If we get there, there will most likely be a "publisher" attribute in the AuthoritaiveGroups (adding being much easier than removing/changing attributes)

djmitche · 2017-08-14T15:53:01Z

cis/schema.json

+            "description": "All email addresses associated with this user's profile. Order is undefined.",
            "items": {
                "type": "object",
                "properties": {


It might be worth adding required and additionalProperties here (and for the other arrays of sub-objects), too.

Add descriptions and some formats to the Profile schema

fed7cc1

djmitche closed this Aug 11, 2017

djmitche reopened this Aug 11, 2017

Add/Correct missing descriptions

e973d85

gdestuynder approved these changes Aug 14, 2017

View reviewed changes

gdestuynder merged commit e973d85 into mozilla-iam:master Aug 14, 2017

djmitche commented Aug 14, 2017

View reviewed changes

djmitche mentioned this pull request Aug 17, 2017

Schema docs, second round #75

Merged

Conversation

djmitche commented Aug 3, 2017

Uh oh!

djmitche commented Aug 7, 2017

Uh oh!

djmitche commented Aug 11, 2017

Uh oh!

djmitche commented Aug 11, 2017

Uh oh!

gdestuynder commented Aug 14, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gdestuynder commented Aug 14, 2017

Uh oh!

djmitche left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gdestuynder Aug 17, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gdestuynder Aug 17, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

gdestuynder commented Aug 14, 2017 •

edited

Loading

gdestuynder Aug 17, 2017 •

edited

Loading

gdestuynder Aug 17, 2017 •

edited

Loading