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

Add SecureAS400.isAdditionalAuthenticationFactorAccepted() #130

Merged
merged 1 commit into from
Sep 18, 2023

Conversation

ThePrez
Copy link
Member

@ThePrez ThePrez commented Sep 13, 2023

No description provided.

@ThePrez ThePrez requested a review from jeber-ibm September 13, 2023 19:15
Copy link
Member

@jeber-ibm jeber-ibm left a comment

Choose a reason for hiding this comment

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

Changes look good. The only disadvantage is the caller has to know about the system configuration to decide whether to used AS400 or SecureAS400. An alternate options is to start 2 threads -- 1 for each port and then interrupt the other thread when a thread gets a response.

Of course, this is probably not an issue for ACS since ACS is told whether to use a secure port.

Signed-off-by: Jesse Gorzinski <17914061+ThePrez@users.noreply.github.com>
@ThePrez
Copy link
Member Author

ThePrez commented Sep 13, 2023

Changes look good. The only disadvantage is the caller has to know about the system configuration to decide whether to used AS400 or SecureAS400. An alternate options is to start 2 threads -- 1 for each port and then interrupt the other thread when a thread gets a response.

Of course, this is probably not an issue for ACS since ACS is told whether to use a secure port.

Yeah, I had the same thought, but concluded that the caller will need to know about the system configuration on the subsequent call to connect. The already-existing AS400 vs SecureAS400 class separation already dictates explicit code legs for the TLS case :/

@ThePrez ThePrez merged commit e96a4e7 into IBM:main Sep 18, 2023
2 checks passed
@ThePrez ThePrez deleted the fix/aaftls1 branch September 18, 2023 18:13
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