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

refactor: Making username and password fields in OidcAuthModel as mandatory onl… #4460

Merged

Conversation

lokeshrangineni
Copy link
Contributor

Added separate model for OidcClient side so that username and password is not required for server side.

What this PR does / why we need it:

class OidcAuthConfig is having username and password as mandatory. OIDC server side code is not using username and password so it is forcing server side code to pass the values. One of the proposal is to create separate model for oidc server and client. We need to evaluate all the util methods if it is going to impact in any other way.

Which issue(s) this PR fixes:

#4457

@lokeshrangineni lokeshrangineni changed the title Making username and password fields in OidcAuthModel as mandatory onl… refactor: Making username and password fields in OidcAuthModel as mandatory onl… Aug 28, 2024
@lokeshrangineni lokeshrangineni changed the base branch from master to feat-documentstore August 29, 2024 16:45
@lokeshrangineni lokeshrangineni changed the base branch from feat-documentstore to master August 29, 2024 16:45
@lokeshrangineni lokeshrangineni marked this pull request as ready for review August 29, 2024 16:50
Copy link
Contributor

@dmartinol dmartinol left a comment

Choose a reason for hiding this comment

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

Please update according to suggested changes

…et fields are required for oidc client configuration

Signed-off-by: Lokesh Rangineni <19699092+lokeshrangineni@users.noreply.github.com>
@lokeshrangineni lokeshrangineni force-pushed the feature/optional-oidc-username-pwd branch from d8e36cf to bb95073 Compare September 3, 2024 19:30
@lokeshrangineni lokeshrangineni changed the base branch from master to v0.23-branch September 3, 2024 19:31
@lokeshrangineni lokeshrangineni changed the base branch from v0.23-branch to master September 3, 2024 19:32
…et fields are required for oidc client configuration

Signed-off-by: Lokesh Rangineni <19699092+lokeshrangineni@users.noreply.github.com>
Signed-off-by: Lokesh Rangineni <19699092+lokeshrangineni@users.noreply.github.com>
@lokeshrangineni
Copy link
Contributor Author

Please update according to suggested changes

I have incorporated all the code review comments. Please take a look again.

Copy link
Contributor

@redhatHameed redhatHameed left a comment

Choose a reason for hiding this comment

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

lgtm

Signed-off-by: Lokesh Rangineni <19699092+lokeshrangineni@users.noreply.github.com>
Signed-off-by: Lokesh Rangineni <19699092+lokeshrangineni@users.noreply.github.com>
Signed-off-by: Lokesh Rangineni <19699092+lokeshrangineni@users.noreply.github.com>
Copy link
Contributor

@dmartinol dmartinol left a comment

Choose a reason for hiding this comment

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

lgtm

Signed-off-by: Lokesh Rangineni <19699092+lokeshrangineni@users.noreply.github.com>
@jeremyary jeremyary removed the request for review from sfc-gh-madkins September 4, 2024 16:04
@jeremyary jeremyary merged commit 3f3a4e8 into feast-dev:master Sep 4, 2024
17 checks passed
@lokeshrangineni lokeshrangineni deleted the feature/optional-oidc-username-pwd branch October 23, 2024 16:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants