Skip to content

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)

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}': #{$!}")

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

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)

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

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

@qlty-cloud-legacy
Copy link

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