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

Backend: Add the availiable userDetails when using the AD-Header approach #1533

Open
jesade-vbg opened this issue Jun 25, 2024 · 4 comments
Open
Assignees
Labels
new feature Request for adding/changing functionality
Projects
Milestone

Comments

@jesade-vbg
Copy link
Contributor

When using the built in AD lookup in the backend you can activate userDetails to get a bunch of stuff from AD related to the user.

When using the AD_USE_GROUPS_FROM_HEADER=true funtionality this is currently not possible.

But.... we could add userName and groups to the userDetails object, and after this PR in NodeHoster you could also get the users email.

Proposal for object in returned config:

userDetails: {
    user: "user from header",
    groups: ["group 1 from header", "group 2....", "group 3....."],
    email: "TheUsersEmail@email.com"
}

Proposal for default env variable:
AD_TRUSTED_EMAIL_HEADER=X-Control-Email-Header

@jesade-vbg jesade-vbg added the new feature Request for adding/changing functionality label Jun 25, 2024
@jesade-vbg jesade-vbg added this to the 3.x milestone Jun 25, 2024
@jesade-vbg jesade-vbg self-assigned this Jun 25, 2024
@jesade-vbg jesade-vbg added this to Approved ideas in Hajk via automation Jun 25, 2024
@jesade-vbg
Copy link
Contributor Author

image

@jesade-vbg jesade-vbg moved this from Approved ideas to In progress in Hajk Jun 26, 2024
@jacobwod
Copy link
Member

To be honest, I'm not sure. It feels like the possible outcome of a getMap request and the structure of the userDetails object will differ a lot between implementations. E.g. here's how it looks on one AD setup:

image

Perhaps we should come up with a common interface for this object?

@jacobwod
Copy link
Member

One thing that comes into mind: if you active to show the button with user initials, what does it say in your setup?

Skärmavbild 2024-06-26 kl  10 49 36

I'm wondering because I notice that I'm utilising the displayName field, as well as the description field, both of which are lacking in your screenshot. Instead you have some fields that I haven't seen before.

As stated previously, I think we should agree upon some interface for userDetails. Perhaps sAMAccountName, mail, displayName and description should always be available?

@jesade-vbg
Copy link
Contributor Author

Actually I have never seen the "button with user initials".

Well, that would be nice. But it would mean even more changes to NodeHoster and even more data will be send on every request through the proxy. Then it will need to be handled in nodebackend as well.

As I cant prioritize this right now and it's not urgent, I would prefer more additions to userDetails as a new issue in both Hajk-repo and NodeHoster-repo. What do you think?

@jesade-vbg jesade-vbg moved this from In progress to Done in Hajk Jul 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature Request for adding/changing functionality
Projects
Hajk
  
Done
Development

No branches or pull requests

2 participants