Skip to content

Remove unused @ial ivar from Users::SessionsController#10083

Merged
jmhooper merged 2 commits intomainfrom
jmhooper-remove-unused-ial-ivar
Feb 14, 2024
Merged

Remove unused @ial ivar from Users::SessionsController#10083
jmhooper merged 2 commits intomainfrom
jmhooper-remove-unused-ial-ivar

Conversation

@jmhooper
Copy link
Contributor

This ivar is part of a conditional that is used to determine if the link to authenticate with a PIV or CAC is visible.

It was added here: ccc40ed

The computation for this value looks like this:

def sp_session_ial
  sp_session[:ial].presence || 1
end

This will always have a truthy value unless the requested IAL is IALMax (the integer value corresponding to IALMax is 0). After reading through the commit history I beleive this IALMax behavior is unintentional.

I believe this value was originally intended to require the user to enter a password if identity proofing was required. It appears this requirement changed but the ivar was left behind. This commit removes it.

This ivar is part of a conditional that is used to determine if the link to authenticate with a PIV or CAC is visible.

It was added here: ccc40ed#diff-cd2d90708a775a9fc605178934b17b6cb5d14d4396b06b916a84c43b1b2abd88R37

The computation for this value looks like this:

```ruby
def sp_session_ial
  sp_session[:ial].presence || 1
end
```

This will always have a truthy value unless the requested IAL is IALMax. After reading through the commit history I beleive this IALMax behavior is unintentional.

I believe this value was originally intended to require the user to enter a password if identity proofing was required. It appears this requirement changed but the ivar was left behind. This commit removes it.

[skip changelog]
@jmhooper jmhooper requested a review from a team February 14, 2024 15:18
Copy link
Contributor

@aduth aduth left a comment

Choose a reason for hiding this comment

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

I think we can remove this as well?

@aduth
Copy link
Contributor

aduth commented Feb 14, 2024

This will always have a truthy value unless the requested IAL is IALMax (the integer value corresponding to IALMax is 0).

Wouldn't 0 also be truthy?

$ irb
> !!0
true

@jmhooper
Copy link
Contributor Author

jmhooper commented Feb 14, 2024

Wouldn't 0 also be truthy?

Yep, my bad. Turns out this did absolutely nothing. I'll fix the PR message when I merge.

@jmhooper jmhooper merged commit 59dfbc6 into main Feb 14, 2024
@jmhooper jmhooper deleted the jmhooper-remove-unused-ial-ivar branch February 14, 2024 16:29
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