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

HTTP UserId Resolver support #2081

Closed
brunocascio opened this issue Mar 12, 2020 · 20 comments · Fixed by #2083
Closed

HTTP UserId Resolver support #2081

brunocascio opened this issue Mar 12, 2020 · 20 comments · Fixed by #2083
Labels
Topic: Resolver Issues concerning user resolver (SQL, LDAP, ...)
Milestone

Comments

@brunocascio
Copy link
Contributor

brunocascio commented Mar 12, 2020

Is your feature request related to a problem? Please describe.
What are you trying to achieve?

Currenlty, pi supports sql, ldap, passwd and scim user's resolvers. A useful case for microservices is retrieving users from an external API. For example, http://domain.com/users/<userId>

Describe the solution you'd like
A clear and concise description of what you want to happen.

  • What is the purpose of the resolver

Use third party HTTP API for retrieving user data without follows the SCIM specs.

  • How it works

Since PI does not store users, it uses resolvers like LDAP, SCIM, SQL, etc. Today, there is no way to resolve user information through an API but SCIM. SCIM uses an authorization server to authenticate the request, HTTP resolver will not. HTTP resolver could authenticate users via Authorization headers instead.

  • How it is configured

The user would create an HTTP resolver only adding an HTTP endpoint under Add httpresolver UI. The endpoint must contain the '%s' symbol inside, symbol where pi will replace with their userId.

Describe alternatives you've considered
A clear and concise description of any alternative solutions or features you've considered.

Add inversion control in order to be the user able to create custom resolvers instead of modifying pi code directly.

Additional context
Add any other context or screenshots, that might help us to better understand your idea, your need and your circumstances.

image

@cornelinux
Copy link
Member

Thanks a lot for your suggestion.
But please note, that a reference to a pull request is NOT a CONSISE description. So I do not actually know, what you are suggesting!

SCIM actually IS an HTTP API to retrieve users. Please describe in more detail, what you want to do, otherwise we will need to close this issue.
Thanks a lot for your understanding.

@brunocascio
Copy link
Contributor Author

brunocascio commented Mar 12, 2020

Hi @cornelinux!

SCIM is for auth servers. My PR is for a simple HTTP API. For example, an external REST user API. http://domain.com/users/<userId>

@cornelinux
Copy link
Member

Sorry. Missing spec or description. We do not do any development without spec.

@brunocascio
Copy link
Contributor Author

brunocascio commented Mar 12, 2020

The user sources is an http API. How do accomplish that today? I've made a pull request with that

@cornelinux
Copy link
Member

Didn't I make myself clear?

We do not accept Pull Requests without knowing what the pull request does.

So you need to tell

  • What is the purpose of the resolver
  • How it works
  • How it is configured.

This is also necessary so that conceptual discussions can take place before code is generated.
Nice, right! This is why privacyIDEA is such a cool piece of software.
If we would have simply trown commits of code on each other, this would be a big pile of crap.

We do documentation and we do specs. You need to accept this and comply to this. Noone will read your code to understand what you want to do!
At the other end of the line users are expecting to get a documented product. We wouldn't tell the read the fucking code.

Read https://github.com/privacyidea/privacyidea/blob/master/CONTRIBUTING.md#develop

@brunocascio
Copy link
Contributor Author

I just follow the template guidelines automatically generated when I made the feature request. I didn't read all the docs.
I know what are you talking about and I can do this.

PD: Relax, we are making software and we are humans.

@cornelinux
Copy link
Member

Thanks.
Appology accepted.

@brunocascio
Copy link
Contributor Author

@cornelinux I have updated the description. Tell me if it is enough for the specs or requires anything more.

@cornelinux
Copy link
Member

cornelinux commented Mar 17, 2020

@brunocascio
So your resolver would simply call

GET https://myuserserver/users/{username}

But what does it return?
Does the "userservice" has to comply to any special data structure?
I would expect some kind of mapping. If you have no mapping you need to describe what the response of the "userservice" has to look like. But then it might be too specific, if it only accepts one response structure ?

So I would expect a description of the request structure and the response structure.

Request

GET https://userservice/users/{userid}

Is it clear if the ID will always be the username, or could the username be allowed to change? I would like this.

Response

Would it be always a json response?

{[
    username: hans,
    surname: wurst,
    givenname: hans,
    email: ....
]}

What if not all attributes are contained - so should be there a mapping?

The thing is, before someone starts to review a code the reviewer should have a functional idea what to expect. A reviewer can not review code, if he does not know, what the code is supposed to do!

Thanks for your patience!

@cornelinux cornelinux reopened this Mar 17, 2020
@cornelinux cornelinux added Status: Waiting for feedback Waiting for more information from the issue creator Topic: Resolver Issues concerning user resolver (SQL, LDAP, ...) labels Mar 24, 2020
@cornelinux
Copy link
Member

@brunocascio Any news on the description of the return values of the mentioned user endpoint?

@brunocascio
Copy link
Contributor Author

Hi @cornelinux,

I'm thinking to close this in favor of refactoring the current users resolvers code so any user would have create their own resolver classes and get it reflected in the UI automatically.

What do you think?

@cornelinux
Copy link
Member

Hi @brunocascio
thanks a lot for this thought. I must admin, we had a very pluggable userresolver config roughly 8 years ago. In this time we had exactly 0 (in words: "zero") resolvers from the community.
During the rewrite we removed this pluggability because it added a lot of additional glue code.
Don't get me wrong, I think it would be great to be independently pluggable. But I would not want this by the costs of too complex code - because it is used like (nearly) never.

So in the first step I would like to understand your resolver and rather add more config possibilities (like defining the attributes or mappings) to this resolver.

@brunocascio
Copy link
Contributor Author

brunocascio commented May 2, 2020

Hi @cornelinux,

Ok, currently I'm using something like this:

image

  1. Resolver name
  2. The endpoint to call (without any template params)
  3. Method (GET or POST)
  4. Request Mapping (body when it is POST or query params when it is GET) In the above example, I set the userid privacyidea user attribute to the "id" in the JSON payload.
  5. Response Mapping. A data mapping structure to match the privacy idea user's fields with the HTTP response values. Nested values could follow dot notation (e.g. phone.mobile).

Example:

  1. A Trigger challenge in fired and its resolved trough the HTTP resolver, so, the PrivacyIDEA HTTPResolver is invoked with { 'userid': 'abc123', ... }
  2. The resolver perform an HTTP request (POST http://example.com/my/endpoint) with the payload { "id": "abc123", "bar": "bar" }

Assuming response status 200 and the following JSON HTTP Response:

{
  "id": "abc123",
  "phone": { "mobile": "+542394567465", "other": null },
  "user_name": "user_example",
  "email": "user@example.com",
  "full_name": "Pepe Perez",
  "first_name": "Pepe",
  "last_name": "Perez",
}
  1. Response mapping will result in the following structure:
{
  "mobile": "+542394567465",
  "username": "user_example",
  "email": "user@example.com"
}

That's the JSON returned by the getUserInfo() method implemented in HTTPResolver class and finally, the user info needed to enroll email and SMS tokens for example.

NOTE:

  • HTTP errors (>= 400) would be thrown as exceptions or/and handled with the configuration from the UI (some non-restful APIS would use 200OK for any case).
  • Requests and responses are JSON.
  • Assume an object as the root of the HTTP JSON response (not an array)

@cornelinux
Copy link
Member

cornelinux commented May 3, 2020

Hi @brunocascio
This sounds reasonable and flexible enough to me. Thanks a lot for the description.
Maybe one day we might want to add HTTP Header attributes, like if the endpoint would require an Authorization header.
But in my opinion, the is a very good start.

So here is our list of

Todo

  • add documentation here
  • Review (and extend where necessary) the code
  • add unit tests like here

@brunocascio Does this sound sensible to you?

@brunocascio
Copy link
Contributor Author

Hey @cornelinux,

Yes, it does. I'll add the header's flags as well, no problem.

Thanks!

@brunocascio
Copy link
Contributor Author

brunocascio commented May 11, 2020

Hi @cornelinux

I bring you updates.

image

image

Special error handling is useful for non-restful where the status code would be 200 but the response "{"success": false}

By the way, HTTP codes >= 400 are thrown as exceptions.

How does it look like for you?

Just in case, there's the code: 20fe49d

@cornelinux
Copy link
Member

Looks promising and now it is easier to understand.
Do you have an example user source implementation to run manually test against it?

Implementing unit tests will be quite easy using responses.

Writing the documentation should also be quite clear now.

@brunocascio
Copy link
Contributor Author

brunocascio commented May 12, 2020

Great! Yes, my next steps are unit testing and documentation now 💃

For the development, I'm using a Mock API built on express.

file: index.js

const app = require('./app')

const app = express().use(express.json());

app.post("/get-2fa-data", function (req, res) {
   return res.json({
       IsError: false,
       Username: req.body.Username,
       Email: 'email-for@2fa.com',
       phone: { mobile: '+542923682293' }
   })
})

app.listen(3000, () => {
  console.log("Running mock server!");
});

Request mapping should be:

{"Username": "{userid}"}

Response mapping would be:

{"userid": "{Email}", "mobile": "{phone.mobile}"}

@brunocascio
Copy link
Contributor Author

brunocascio commented Jun 7, 2020

Hey @cornelinux,

I have finished the work! Let me know if it is OK for the privacyidea team.

Cheers!

@cornelinux cornelinux added this to the 3.4 milestone Aug 18, 2020
@laclaro laclaro added this to To do in privacyIDEA 3.4 via automation Aug 27, 2020
@laclaro laclaro removed the Status: Waiting for feedback Waiting for more information from the issue creator label Aug 27, 2020
@cornelinux cornelinux moved this from To do to nearly Done in privacyIDEA 3.4 Sep 4, 2020
@cornelinux cornelinux moved this from nearly Done to Done in privacyIDEA 3.4 Sep 4, 2020
@rpirsc13
Copy link

How is this helpful and how should it be used ?

Fair enugh, the resolver config works at getting the test user but how do you actually assign tokens to users from this resolver ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Topic: Resolver Issues concerning user resolver (SQL, LDAP, ...)
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

4 participants