8000 Allow OIDC Providers to be available via local socket by jvanderhoof · Pull Request #2616 · cyberark/conjur · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Allow OIDC Providers to be available via local socket #2616

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

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

jvanderhoof
Copy link
Contributor
@jvanderhoof jvanderhoof commented Jul 29, 2022

Desired Outcome

The outcome of this PR is to provide a mechanism for a local service to retrieve a list of configured OIDC authenticators.

Note
This functionality is intended as a stop-gap for the UI in Conjur Enterprise. The ui socket service will be removed in the near future.

Implemented Changes

This PR includes a couple of changes:

  • Refactor the authn-local unix socket server to accept a custom response.
  • Enable the /:authenticator/:account/providers route to be served over a local unix socket.
  • Refactors authn-local to utilize the generic unix socket service

Connected Issue/Story

CyberArk internal issue link: ONYX-23542

Definition of Done

At least 1 todo must be completed in the sections below for the PR to be
merged.

Changelog

  • The CHANGELOG has been updated, or
  • This PR does not include user-facing changes and doesn't require a
    CHANGELOG update

Test coverage

  • This PR includes new unit and integration tests to go with the code
    changes, or
  • The changes in this PR do not require tests

Note

  • `authn-local service is tested at the integration level
  • The behavior of the Authentication::AuthnOidc::V2::Views::ProviderContext class is well tested with unit tests.
  • Additional tests need to be added in the near future.

Documentation

  • Docs (e.g. READMEs) were updated in this PR
  • A follow-up issue to update official docs has been filed here: insert issue ID
  • This PR does not require updating any documentation

Behavior

  • This PR changes product behavior and has been reviewed by a PO, or
  • These changes are part of a larger initiative that will be reviewed later, or
  • No behavior was changed with this PR

Security

  • Security architect has reviewed the changes in this PR,
  • These changes are part of a larger initiative with a separate security review, or
  • There are no security aspects to these changes

@jvanderhoof jvanderhoof force-pushed the provider-list-via-socket branch from 98bd22c to 368531e Compare July 29, 2022 18:51
# message: passed_arguments
# )
# end
def run(&block)
Copy link

Choose a reason for hiding this comment

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

Method run has a Cognitive Complexity of 7 (exceeds 5 allowed). Consider refactoring.

begin
connection.puts(block.call(arguments))
rescue
@message_writer.puts("Error in service '#{@socket}': #{$!}")
Copy link

Choose a reason for hiding this comment

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

Prefer $ERROR_INFO from the stdlib 'English' module (don't forget to require it) over $!.


@message_writer.puts("service is listening at #{@socket}")

while connection = server.accept
Copy link

Choose a reason for hiding this comment

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

Use == if you meant to do a comparison or wrap the expression in parentheses to indicate you meant to assign in a condition.

# message: passed_arguments
# )
# end
def run(&block)
Copy link

Choose a reason for hiding this comment

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

Complex method Util::SocketService#run (25.8)

module Authentication
module AuthnOidc
module V2
module Commands
Copy link

Choose a reason for hiding this comment

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

Authentication::AuthnOidc::V2::Commands has no descriptive comment

@codeclimate
Copy link
codeclimate bot commented Jul 29, 2022

Code Climate has analyzed commit 368531e and detected 11 issues on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 6
Style 5

The test coverage on the diff in this pull request is 34.6% (50% is the threshold).

This pull request will bring the total coverage in the repository to 89.7% (-1.7% change).

View more on Code Climate.

This commit includes an a rework of the authn-local socket server to enable
a secondary local socket (intended for Conjur UI) to deliver the list of available
OIDC Providers to the UI.

This work is a temporary stopgap. It will be removed when partial replication (Conjur
Enterprise) has been completed.
This is a refactor of authn-local to leverage the generic Socket Server, which
is used by the ui service.
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.

1 participant
0