Skip to content

Conversation

@lukasmatusiewicz
Copy link
Contributor

@lukasmatusiewicz lukasmatusiewicz commented Nov 13, 2025

This pull request refactors the privacyIDEAADFSProvider/Adapter.cs file to centralize and standardize string constants by using the PIConstants class, and modularizes authentication logic for better maintainability. The changes improve code readability, reduce hard-coded strings, and make the authentication flow easier to extend and debug.

Refactoring for constant usage:

  • Replaced hard-coded string literals throughout the file with named constants from PIConstants, including keys for context data, transaction IDs, modes, error messages, and HTTP headers. This makes the code less error-prone and easier to maintain. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11]

Modularization of authentication logic:

  • Extracted authentication steps for Passkey, Push, and WebAuthn into dedicated helper methods (ProcessPasskeyAuthentication, ProcessPasskeyRegistration, ProcessPushAuthentication, ProcessWebauthnAuthentication). This reduces code duplication and improves clarity in the main authentication flow. [1] [2]

General improvements:

  • Updated the default values and error handling to use constants, ensuring consistent messaging and easier updates in the future.
  • Improved the construction of the PrivacyIDEA client by using a constant for the user agent string.

@lukasmatusiewicz lukasmatusiewicz linked an issue Nov 13, 2025 that may be closed by this pull request
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request refactors the privacyIDEAADFSProvider codebase to improve maintainability and add support for forwarding custom client information during authentication. The changes standardize string usage through named constants, enhance parameter forwarding capabilities, and update dependencies.

Key changes:

  • Introduced PIConstants class to replace hardcoded strings throughout the codebase
  • Added configuration flags and logic to forward client IP address and user agent to the PrivacyIDEA backend
  • Enhanced enrollment cancellation handling with new UI controls and API support

Reviewed Changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
PIConstants.cs New file defining constants for endpoints, parameters, token types, and other string literals
PrivacyIDEA.cs Refactored to use PIConstants; added custom parameter forwarding to all API methods
PIResponse.cs Updated to use constants and added enrollment via multichallenge optional flags
Adapter.cs Implemented custom parameter collection and forwarding; added enrollment cancellation handling
Configuration.cs Added configuration flags for forwarding client IP and user agent
AuthPage.html Added cancel enrollment button and supporting JavaScript
AdapterPresentationForm.cs Added property for enrollment via multichallenge optional state
FormResult.cs Added enrollment cancelled flag
RegistryReader.cs, PILog.cs, PIChallenge.cs, PIEnrollResponse.cs, PIWebAuthnSignRequest.cs Updated namespace references
Tests.csproj Added Newtonsoft.Json dependency
PrivacyIDEAClientTests.cs Renamed class and updated namespace import
privacyIDEAADFSProvider.sln Updated Visual Studio version

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@lukasmatusiewicz lukasmatusiewicz linked an issue Nov 13, 2025 that may be closed by this pull request
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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.

Transform strings to consts enroll via multichallenge optional

2 participants