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

domain scoped authentication with tests #503

Merged
merged 1 commit into from
Nov 15, 2015
Merged

Conversation

auhlig
Copy link
Member

@auhlig auhlig commented Nov 9, 2015

This PR related to issue Domain scoped authentication #497 and enables OpenStack4j to authenticate with a domain scope in the identity v3 case.
This is the last commit for this kind of issues which is compatible to the v2 implementation.

@auhlig auhlig added this to the 2.0.9 Release milestone Nov 9, 2015
@auhlig
Copy link
Member Author

auhlig commented Nov 9, 2015

Hey @mazimo,
Would you mind taking a look?
BR


return new V2Token(id, token.getExpires(), token.getVersion(), tenant);
}
else if(token.getDomain() != null) {
Copy link
Member

Choose a reason for hiding this comment

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

Probably could break this out of a nested else sequence. ie. if (token.get...) since the previous condition is returning a token

Copy link
Member Author

Choose a reason for hiding this comment

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

Right. But if you don't mind I would leave it that way for now and do better on the new dedicated v3 branch :)

@gondor
Copy link
Member

gondor commented Nov 9, 2015

Looks good @auhlig from an implementation standpoint. LGTM

@auhlig
Copy link
Member Author

auhlig commented Nov 13, 2015

If there aren't any further objections or comments by @mazimo or @gondor ,maybe we could merge and include this in the next release?

@gondor
Copy link
Member

gondor commented Nov 13, 2015

👍 LGTM

gondor added a commit that referenced this pull request Nov 15, 2015
domain scoped authentication with tests
@gondor gondor merged commit 9396678 into ContainX:master Nov 15, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants