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

Implement the security endpoints check for identification #783

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

okyame
Copy link

@okyame okyame commented Jan 12, 2019

close #782

@okyame
Copy link
Author

okyame commented Jan 12, 2019

No more need of PR #620

sys.path.insert(0, "..")


def simple_certificate_manager(isession, certificate, signature):
Copy link
Member

Choose a reason for hiding this comment

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

I have not looked in detail at this. But is it enough with a function? Should'nt we use a class with a few methods? just asking

Copy link
Member

Choose a reason for hiding this comment

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

if it is just a function, then using the name manager is a bit wrong. A manager is an object. a function would be called something like set_certificate_validation_function or whatever


# FIXME: Get the server available UserIdentityToken policy's ids. I don't how to get it in other way.
# iserver_policy_ids = ["anonymous", "certificate_basic256sha256", "username"]
# Note that it is different from server._policyIDs = ["Anonymous", "Basic256Sha256", "Username"]
Copy link
Member

Choose a reason for hiding this comment

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

yes that seems to be the correct way to get policy IDs . Just write a comment that we assume that the same polcies are used n all endpoints.. maybe add a FIXME. I am not sure what spec says but in theory it should be possible to have different policies per endpoints


# Route to the correct manager
if isinstance(params_id_token, ua.AnonymousIdentityToken):
pass # TODO: Call AnonymousIdentityToken Manager
Copy link
Member

Choose a reason for hiding this comment

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

we should at least check that "anonymous" is in the list of Policies and return an UaError if not

Copy link
Member

Choose a reason for hiding this comment

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

Can't we use the user_manager for that? maybe just have a bool member called something like "allow_anonymous"? not sure I have not looked at that code for very long...

@oroulet
Copy link
Member

oroulet commented Jan 12, 2019

thanks for the patch anyway. this looks good

@oroulet
Copy link
Member

oroulet commented Feb 7, 2019

@okyame what is the status?

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

Successfully merging this pull request may close these issues.

How to implement the security ID check
2 participants