Skip to content
This repository was archived by the owner on Aug 28, 2024. It is now read-only.

Apollo-1116- updated docs with XML examples - #68

Open
anuritjain wants to merge 7 commits into
rackerlabs:masterfrom
anuritjain:anurit/apollo-1116-updating-XML-examples
Open

Apollo-1116- updated docs with XML examples#68
anuritjain wants to merge 7 commits into
rackerlabs:masterfrom
anuritjain:anurit/apollo-1116-updating-XML-examples

Conversation

@anuritjain

@anuritjain anuritjain commented Jul 30, 2020

Copy link
Copy Markdown

Add XML mapping policies to federation docs

  • Apollo Dev Sign-off
  • Doc Teams Sign-off

@rackernexus

Copy link
Copy Markdown

Build 142 was successful. Staging content Preview!. valid for 14 days

Comment on lines +122 to +123
<attribute path="(\n if (mapping:get-attributes('groups')='rackspace-billing')\n then 'billing:admin' else ()\n)\n"
multiValue="true"/>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you replace the \n characters with actual new lines?
The goal of this comment, is to format the path attribute in a way that is is most readable to customers:

Suggested change
<attribute path="(\n if (mapping:get-attributes('groups')='rackspace-billing')\n then 'billing:admin' else ()\n)\n"
multiValue="true"/>
<attribute
path="(
if (mapping:get-attributes('groups')='rackspace-billing')
then 'billing:admin' else ()
)"
multiValue="true"
/>

Before making this change across all the examples I would make sure it validates first, but I think it will make the examples easier to read the conditional logic in attributes is allowed to span multiple lines.

Comment thread doc/overview/features.rst Outdated
credentials instead of requiring new credentials.
* - Provides attribute mapping policy capability
- Uses a rich, YAML-based, policy language that enables you to dynamically
- Uses a rich, XML-based, policy language that enables you to dynamically

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- Uses a rich, XML-based, policy language that enables you to dynamically
- Uses a rich, YAML-based or XML-based, policy language that enables you to dynamically

@rackernexus

Copy link
Copy Markdown

Build 143 was successful. Staging content Preview!. valid for 14 days

@rackernexus

Copy link
Copy Markdown

Build 144 was successful. Staging content Preview!. valid for 14 days

Comment thread doc/manage-idps/index.rst Outdated

The file must be valid YAML, and the file extension should be ``.yml`` or
``.yaml``. To validate your YAML, you can use any YAML validation library or
The file must be valid XML, and the file extensions should be ``.xml``. To validate your XML, you can use any XML validation library or

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
The file must be valid XML, and the file extensions should be ``.xml``. To validate your XML, you can use any XML validation library or
The file must be valid XML or YAML, and the file extensions should be ``.xml`` or ``.yml`` . To validate your document, you can use any XML or YAML validation library or

Comment thread doc/appendix/examples.rst Outdated
<user>
<domain value="{D}"/>
<name value="{D}"/>
<email value="{At(FirstName)} {At(LastName)} &lt;{D(name)}@{D(domain)}.rackspace.com&gt;"/>

@nicko-winner nicko-winner Jul 31, 2020

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verify that &lt; and &gt; are the appropriate character to use here, check the federation test case examples and see if they encode these characters <> in the same way. (I know this kind of encoding is needed for <> character being used inside of a tag, not sure if its needed inside an attribute)

@nicko-winner nicko-winner Jul 31, 2020

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment thread doc/appendix/map.rst Outdated
<domain value="{D}"/>
<name value="{D}"/>
<email value="{D}"/>
<roles value="{D}" multiValue="true"/>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is no multiValue="true" in the YAML example, is this needed if you are using a default value here {d}?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when i converted Yaml to XML using CLI tool, it is giving me same output, i.e, with multivalue="true", so kept it as it is..

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the other examples do not seem to be using multiValue="true" when using a default value on roles, we should probably be consistent here.

https://github.com/rackerlabs/docs-rackspace-federation/pull/68/files#diff-e684b4a76886438d5d307093da10b392R198

I will check the test cases, but i'm not sure multiValue="true" is getting converted consistently, or as simply as it could be.

@rackernexus

Copy link
Copy Markdown

Build 145 was successful. Staging content Preview!. valid for 14 days

Comment thread doc/appendix/examples.rst
xmlns="http://docs.rackspace.com/identity/api/ext/MappingRules"
version="RAX-1">
<rules>
<rule>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason for tag in all these XML examples? This is not in YAML

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question. I wonder if @nikhil09541 , or Tim Cline may have more context as to why it is needed as this tag seems to be an identity requirement and is used in the test cases repo.

@nicko-winner nicko-winner Aug 4, 2020

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment thread doc/appendix/intro.rst
Comment on lines -26 to -29
- **YAML 1.1**: YAML is a simple data serialization language that is designed
to be human friendly. YAML is very similar to JSON but allows for useful
features such as comments and the ability to easily input multi-line data.
Attribute mapping policies are written in YAML.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

YAML 1.1 section should be intact as we will be supporting both YAML and XML. May be we should update this section to call out XML as well. Thoughts?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

restored YAML 1.1 section and added XML 1.0 section below it.

Comment thread doc/appendix/intro.rst Outdated
<domain value="{D}"/>
<name value="{D}"/>
<email value="{At(email)}"/>
<roles value="{D}"/>

@prathibha-emany prathibha-emany Aug 3, 2020

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

<roles value="{0}" in the YAML example. In this XML example it is noted as {D}, Can you please check this ? https://developer.rackspace.com/docs/rackspace-federation/appendix/intro/#mapping-policy-for-widget-com

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed.

Comment thread doc/appendix/map.rst Outdated
<name value="{Pts(/saml2p:Response/saml2:Assertion/saml2:Subject/saml2:NameID)}"/>
<email value="{Pts(/saml2p:Response/saml2:Assertion/saml2:AttributeStatement/saml2:Attribute[@Name='email']/saml2:AttributeValue[1])}"/>
<roles value="{Pts(/saml2p:Response/saml2:Assertion/saml2:AttributeStatement/saml2:Attribute[@Name='roles']/saml2:AttributeValue)}"
multiValue="true"/>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see in many instances , roles Value tag is ending with multiValue=true which is not in YAML samples. Can we look into why it is appearing?

@anuritjain anuritjain Aug 4, 2020

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for this, I have checked & converted all the examples again and found that in most of the examples, multiValue="true" is adding automatically in <roles value tag.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found a good multi value example here: https://github.com/rackerlabs/attributeMapping/blob/1aa04de04e01fc6ff916f1b00fa2cead24305390/core/src/test/resources/tests/mapping-tests/adfs-faws-ext1/maps/adfs-single-rule.xml

It seems like if you are not trying to map multiple attributes to a single field than you do not need multiValue="true"

Comment thread doc/appendix/map.rst
Parts of the mapping policy
~~~~~~~~~~~~~~~~~~~~~~~~~~~

The mapping policy is a **YAML** document that contains instructions to

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably keep both YAML and XML references until we completely deprecate YAML

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done.

</user>
</local>
<remote>
<attribute

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is this <attribute tag that is not in YAML, was it just added during conversion as <attribute path ?

@anuritjain anuritjain Aug 4, 2020

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when we convert Yaml to XML, this tag is added under <remote tag, as path is an attribute of remote.

<roles value="{0}" multiValue="true"/>
<expire value="PT4H"/>
</user>
<faws xsi:type="LocalAttributeGroup">

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this "LocalAttributeGroup"? Unable to locate this in YAML

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is the best example I could find that offered some context on this:

ref: https://github.com/rackerlabs/attributeMapping/blob/1aa04de04e01fc6ff916f1b00fa2cead24305390/core/src/test/resources/tests/mapping-tests/mapping-rule-ext-attribute/maps/mapping-rule-ext-attribute.xml#L28-L37

From what I am seeing in the identity tests repo, if there is a <faws tag it will also have LocalAttributeGroup, looks like a requirement of the schema

<roles value="{0}" multiValue="true"/>
<expire value="PT4H"/>
</user>
<faws xsi:type="LocalAttributeGroup">

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this "LocalAttributeGroup" value? Unable to locate this in YAML examples

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems that for faws and aws tags, this xsi:type namespace is used in some examples.

Comment thread doc/appendix/intro.rst Outdated
Comment on lines +172 to +179
for $project in mapping:get-attributes('manager_projects') return
(
if ($project = 'widgets_ui') then 'admin/777654' else
if ($project = 'widgets_mobile') then 'admin/887655' else
if ($project = 'widgets_platform') then 'admin/779956' else
()
)
) else

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would it be possible to match the indentation approach used in the YAML file? Seems like the 'if' statements should be inside of the forloop, adding indentation would make it more apparent. I can't say i'm very familiar with the style characteristics of this language, but if we can use the YAML file as a reference. Might be worth referring to the YAML docs for how to deal with spacing here.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed indentation as per YAML example.

@rackernexus

Copy link
Copy Markdown

Build 146 was successful. Staging content Preview!. valid for 14 days

@catlook catlook left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Comment thread doc/appendix/examples.rst Outdated
xmlns="http://docs.rackspace.com/identity/api/ext/MappingRules"
version="RAX-1">
<rules>
<rule>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you indent <rule> here so it looks like its inside `

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Comment thread doc/appendix/examples.rst Outdated
version="RAX-1">
<rules>
<rule>
<local>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you indent <local />

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Comment thread doc/appendix/map.rst Outdated
<description>The default policy. All attributes are in the expected location in the SAML assertion.</description>
<rules>
<rule>
<local>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indent

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Comment thread doc/appendix/intro.rst Outdated
version="RAX-1">
<description>The following is an attribute mapping for Widgets.com.</description>
<rules>
<local>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all the other examples have a <rules> tag with a<rule> tag inside it, Is that not a required convention in this case?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for this example, after the conversion, <rule> tag is not added in the code by default, if u want I can add manually now.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I would be nice to know in what cases the <rule> attribute is not needed. Maybe @nikhil09541 can provide context here. We can also try to do some testing on the apollodevuser IDP to how identity responds to this format.

version="RAX-1">
<rules>
<rule>
<local>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indent local

version="RAX-1">
<rules>
<rule>
<local>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indent local

version="RAX-1">
<rules>
<rule>
<local>

@nicko-winner nicko-winner Aug 6, 2020

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indent local, can you check the other examples and make sure they are all indented correctly? there are at least a few others that could use indentation.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have added indentation to all examples at <local> tag and looks fine now. please check once.

@rackernexus

Copy link
Copy Markdown

Build 148 was successful. Staging content Preview!. valid for 14 days

<domain value="636462353"/>
<name value="{D}"/>
<email value="{At(http://schemas.xmlsoap.org/ws/2005/05/identity/claims/emailaddress)}"/>
<roles value="nova:observer lbaas:admin" multiValue="true"/>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have tested the xml provided here , it fails and gives 400 error due to the multiline value="true".
Ideally shouldn't we provide the multiple values to single attribute like this:

           <roles multivalue="true">
           <role value='admin'/>
           <role value='billing:admin'/>
           <role value='ticketing:admin'/>
            <roles/>

Please cross check with the xsd schema which currently is a blueprint for xml here :
https://github.com/rackerlabs/attributeMapping/blob/master/core/src/main/resources/xsd/mapping.xsd#L93-L107


<?xml version="1.0" encoding="UTF-8" ?>
<mapping xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xmlns:xs="http://www.w3.org/2001/XMLSchema"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The format of the policy fails if we add new line over here . Accepted format includes xml declaration in a single line like :

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

<mapping xmlns="http://docs.rackspace.com/identity/api/ext/MappingRules" version="RAX-1" xmlns:saml2="urn:oasis:names:tc:SAML:2.0:assertion" xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"> <rules> <rule> <local> <user> <name value="{0}"/> <email value="{1}"/> <expire value="PT2H"/> <domain value="{2}"/> <roles value="{3}"/> </user> </local> <remote> <attribute path="/saml2p:Response/saml2:Assertion/saml2:Subject/saml2:NameID"/> <attribute path="mapping:get-attribute('email')"/> <attribute path="mapping:get-attribute('domain')"/> <attribute multiValue="true" path="mapping:get-attributes('roles')"/> </remote> </rule> </rules> </mapping>

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants