Skip to content

Conversation

@nitschSB
Copy link
Contributor

I added an ldap user service that connects to the an ldap server.

For every ldap user that logs in the first time, a vrt user is created with editor role.

Visual-Regression-Tracker/#324

@nitschSB nitschSB changed the title Feature: add ldap support [WIP] Feature: add ldap support Jul 17, 2024
@nitschSB nitschSB force-pushed the feature/ldap branch 3 times, most recently from 409cb8f to ceca1f2 Compare July 17, 2024 12:05
@nitschSB nitschSB changed the title [WIP] Feature: add ldap support Feature: add ldap support Jul 17, 2024
@nitschSB
Copy link
Contributor Author

Hi, now it's working as expected for me.
Cheers

@pashidlos
Copy link
Member

@nitschSB thanks for contirubtion!
I'll try to review it ASAP, currently have electricity for 4h a day )
could you meanwhile share some setup for local ldap server or how you tested it?

@nitschSB
Copy link
Contributor Author

I added some local test setup. have a look at the README.md

I'm not familiar with the frameworks you use. I got this done with the help of copilot and try&error. I think some automated tests would be nice, but my attempts were not successful - sorry for the bad test coverage.

@nitschSB
Copy link
Contributor Author

Hi @pashidlos , do you have any questions about this? Is there anything you want me to do before merging?

@nitschSB nitschSB force-pushed the feature/ldap branch 3 times, most recently from 4c4ab3d to 66b3037 Compare January 15, 2025 13:57
@nitschSB nitschSB force-pushed the feature/ldap branch 2 times, most recently from 8d11cd5 to 66c3809 Compare January 15, 2025 16:36
@nitschSB
Copy link
Contributor Author

Hi @pashidlos ,

I did my best to adopt the factory pattern, like you did for storage.

I also added a e2e with ldap test to the .github/workflow.yml pipeline.

Looking forward to see a merge soon ;)

Cheers
Mario

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.

2 participants