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

Allow login to WebUI using Kerberos aliases/enterprise principals #420

Closed
wants to merge 1 commit into from

Conversation

martbab
Copy link
Contributor

@martbab martbab commented Jan 30, 2017

The logic of the extraction/validation of principal from the request and
subsequent authentication was simplified and most of the guesswork will be done
by KDC during kinit. This allows for using principal aliases/enterprise
principals to log into WebUI and also lays foundation for enabling logons from
users from trusted domains.

https://fedorahosted.org/freeipa/ticket/6343

@ghost ghost self-assigned this Feb 2, 2017
@ghost
Copy link

ghost commented Feb 2, 2017

LGTM and works as expected. Not ACKing only because it's marked as WIP.

@@ -977,7 +960,7 @@ def __call__(self, environ, start_response):
# Get the ccache we'll use and attempt to get credentials in it with user,password
ipa_ccache_name = get_ipa_ccache_name()
try:
self.kinit(user, self.api.env.realm, password, ipa_ccache_name)
self.kinit(unicode(user_principal), password, ipa_ccache_name)
Copy link

Choose a reason for hiding this comment

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

Nitpick: unicode is not necessary, kinit() does the conversion anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh okay I missed that.

@martbab
Copy link
Contributor Author

martbab commented Feb 3, 2017

I would wait with ACKing/pushing this PR until #314 is pushed.

@martbab martbab changed the title WIP: Allow login to WebUI using Kerberos aliases/enterprise principals Allow login to WebUI using Kerberos aliases/enterprise principals Feb 28, 2017
@martbab
Copy link
Contributor Author

martbab commented Feb 28, 2017

Now that privilege separation was implemented I have rebased the PR and request a proper review of this patch.

@martbab martbab requested review from abbra and simo5 February 28, 2017 12:54
Copy link
Contributor

@simo5 simo5 left a comment

Choose a reason for hiding this comment

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

LGTM

@martbab
Copy link
Contributor Author

martbab commented Mar 6, 2017

@abbra can you also have a quick look at this PR if it is OK from the trusted user login perspective?

# username is of the form user or of some wild form, e.g.
# user@REALM1@REALM2 or NetBIOS1\NetBIOS2\user (see normalize_name)
# user@REALM1@REALM2 or NetBIOS1\NetBIOS2\user
Copy link
Contributor

Choose a reason for hiding this comment

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

The comment is not true because ipapython.kerberos.Principal does not expect NetBIOS1\user, it only expects normal Kerberos principals.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's true I will fix that.


# wild form username will fail at kinit, so nothing needs to be done
pass
if not (user_principal.is_user or user_principal.is_enterprise):
Copy link
Contributor

Choose a reason for hiding this comment

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

This code actually prevents services to contact us. you should also allow user_principal.is_service.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does it make sense for a service to use WebUI? Or is the login_kerberos handler used anywhere else? AFAIK it handles only WebUI logins currently.

Copy link
Contributor

Choose a reason for hiding this comment

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

login_kerberos can be used by any API user, e.g. any Kerberos principal. I'd prefer to not remove this possibility.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for catching this, I have not realized that. I will then remove all restrictions apart from being parseable principal name.

Copy link
Contributor

@abbra abbra left a comment

Choose a reason for hiding this comment

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

I left few comments in places where I want changes.

@martbab
Copy link
Contributor Author

martbab commented Mar 7, 2017

@abbra I have a question regarding one of your comments, please review.

The logic of the extraction/validation of principal from the request and
subsequent authentication was simplified and most of the guesswork will
be done by KDC during kinit. This also allows principals from trusted
domains to login via rpcserver.

https://fedorahosted.org/freeipa/ticket/6343
@abbra
Copy link
Contributor

abbra commented Mar 8, 2017

Thanks. LGTM and works for me with IPA user, IPA host principal, and AD user. The latter cannot yet actually use Web UI but that is a separate PR.

@abbra abbra added the ack Pull Request approved, can be merged label Mar 8, 2017
@ghost ghost added the pushed Pull Request has already been pushed label Mar 8, 2017
@ghost
Copy link

ghost commented Mar 8, 2017

master:

  • f8d7e37 Allow login to WebUI using Kerberos aliases/enterprise principals

@ghost ghost closed this Mar 8, 2017
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ack Pull Request approved, can be merged pushed Pull Request has already been pushed
Projects
None yet
3 participants