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

modify SCIM plugin #106

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

modify SCIM plugin #106

wants to merge 27 commits into from

Conversation

Otijom
Copy link
Collaborator

@Otijom Otijom commented Mar 15, 2024

No description provided.

@Otijom Otijom marked this pull request as ready for review March 15, 2024 08:46
.. important::
The SCIM API endpoint provided by the plugin must be accessible from the identity provider. If we talk about Azure or Okta, this particular url should be available from the internet. We suggest strongly to limit the ip addresses that can access this url (in addition of adding a strong authentication method).

A Note about passwords sync
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this section be placed on its file?
Generation of readthedoc makes awkward the result: https://glpi-plugins--106.org.readthedocs.build/en/106/scim/requirements.html#a-note-about-passwords-sync

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The fact that the URL is available on the Internet seems to me to be a prerequisite, I can revise the layout if it doesn't suit.
As far as password sync is concerned, I can make a dedicated page for that.

source/scim/okta.rst Outdated Show resolved Hide resolved
Comment on lines -26 to -28
For Azure, the awaited secret is a long life valid jwt token.
We cannot use an oauth exchange (Azure doesn’t ask for an authorize URL).
So in GLPI, setup you SCIM server with **Bearer** security and paste the JWT token from GLPI in the **Secret token** field of Azure.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we lose this important information when migrating to "Entra" document.
As we can't use a real OAuth exchange with this provider, we must explicitly say it.
The current "Make sure you paste the token (Jwt token) to ensure your application works properly." is not sufficient.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok, I can also put this sentence in the GLPI configuration
image
Do you think it's better this way?

Copy link
Contributor

Choose a reason for hiding this comment

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

No, I want the full sentence like before. The last is too simple and does not explain why we need a long-life JWT token (it's a bad practice to have this kind of token as they can be stolen)

@Otijom Otijom requested a review from orthagh March 18, 2024 08:59
source/scim/entra.rst Outdated Show resolved Hide resolved
source/scim/entra.rst Outdated Show resolved Hide resolved
source/scim/entra.rst Outdated Show resolved Hide resolved
source/scim/entra.rst Outdated Show resolved Hide resolved
source/scim/entra.rst Outdated Show resolved Hide resolved
source/scim/password_SSO.rst Outdated Show resolved Hide resolved
source/scim/requirements.rst Outdated Show resolved Hide resolved
source/scim/setup_plugin.rst Outdated Show resolved Hide resolved
source/scim/setup_plugin.rst Outdated Show resolved Hide resolved
source/scim/setup_plugin.rst Outdated Show resolved Hide resolved
Otijom and others added 19 commits March 27, 2024 16:21
Co-authored-by: Curtis Conard <cconard96@gmail.com>
Co-authored-by: Curtis Conard <cconard96@gmail.com>
Co-authored-by: Curtis Conard <cconard96@gmail.com>
Co-authored-by: Curtis Conard <cconard96@gmail.com>
Co-authored-by: Curtis Conard <cconard96@gmail.com>
Co-authored-by: Curtis Conard <cconard96@gmail.com>
Co-authored-by: Curtis Conard <cconard96@gmail.com>
Co-authored-by: Curtis Conard <cconard96@gmail.com>
Co-authored-by: Curtis Conard <cconard96@gmail.com>
Co-authored-by: Curtis Conard <cconard96@gmail.com>
Co-authored-by: Curtis Conard <cconard96@gmail.com>
Co-authored-by: Curtis Conard <cconard96@gmail.com>
Co-authored-by: Curtis Conard <cconard96@gmail.com>
Co-authored-by: Curtis Conard <cconard96@gmail.com>
Co-authored-by: Curtis Conard <cconard96@gmail.com>
Co-authored-by: Curtis Conard <cconard96@gmail.com>
Co-authored-by: Curtis Conard <cconard96@gmail.com>
Co-authored-by: Curtis Conard <cconard96@gmail.com>
Co-authored-by: Curtis Conard <cconard96@gmail.com>
Otijom and others added 4 commits March 27, 2024 16:26
Co-authored-by: Curtis Conard <cconard96@gmail.com>
Co-authored-by: Curtis Conard <cconard96@gmail.com>
Co-authored-by: Curtis Conard <cconard96@gmail.com>
Co-authored-by: Curtis Conard <cconard96@gmail.com>
@orthagh
Copy link
Contributor

orthagh commented Mar 28, 2024

Additionnal comment:
image

The setup of GLPI should be in first, no? You need to paste a token from GLPI when creating the application in the identity provider.

Copy link
Contributor

@trasher trasher left a comment

Choose a reason for hiding this comment

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

Transifex configuration must be adapted.

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.

4 participants