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

wrong password attempt via webui is not logged #5525

Open
gowrisankar22 opened this issue Apr 30, 2020 · 7 comments
Open

wrong password attempt via webui is not logged #5525

gowrisankar22 opened this issue Apr 30, 2020 · 7 comments

Comments

@gowrisankar22
Copy link

Hello Colleagues,
Moving the issue from concourse-chart to here.

I get the log entry if I do failed login via fly commands but Not via concourse web ui.
This seems to be an issue.

More details.
concourse/concourse-chart#65

BRs, gowrisankar

@gowrisankar22
Copy link
Author

@pivotal-jamie-klassen Can you help ?

@jamieklassen
Copy link
Member

jamieklassen commented May 6, 2020

It's nice to log wrong password attempts, but it's never a feature that concourse has advertised - in fact the only reason you see a log for fly login is a coincidence: fly automatically hits the token endpoint regardless of login success/failure. And I don't even think it will do this any more as of 6.1.0! 😬 @pivotal-jwinters could confirm.

All this to say, I don't think I would consider this a bug - the fact is that dex itself doesn't even log wrong password attempts:

https://github.com/dexidp/dex/blob/0a85a97ba9d85fa0de8542a68f1edbc53ed77c25/server/handlers.go#L378-L383

However, I think logging wrong password attempts would be a nice feature. But it might not be feasible.

In fact, dexidp/dex#1671 suggests that username/password logins are generally not production-ready. They might accept a PR adding the kind of logging you're requesting, but they might also not want to take on the maintenance burden of "production-ready username/password logins" - which is a bit of an oxymoron 😆.

Also if you wanted to submit a PR to concourse which implemented this feature without touching the dex codebase, I'm sure we'd consider it, but I can imagine the implementation being pretty tricky.


Now that I've said all that - why do you want this logging? I could imagine it being some security policy in your company, but if you've got a security mandate you probably shouldn't have local users at all. If you need local users it's probably because you're doing some exotic automation, and that takes us back to #3208 which is a whole can of worms!

@g-fusion
Copy link

Hello @jamieklassen,

This issue is still valid for us, for Bosh deployment as well. I will try to provide more info:

Setup:

  • Bosh deployment
  • Concourse is configured with CF/UAA auth
  • Concours local user auth is enabled as well to provide one/single "technical user"
  • fly cli is used to interact with Concourse

Use case:

  • we have large number or Concourse instances exposed externally
  • fly cli is used by automation and with above "technical user" to mange pipelines i.e. upload new, delete some, extract info about pipelines and teams, etc.. Things that we cannot do inside Concourse
  • no LDAP integration is possible

Here is the:

  1. security issue
  • Once local user auth is enabled login page in the UI shows options to login via basic auth (user/pass)
  • Brute force is possible for "technical user"
  • fly cli does not give much flexibility to use other methods to login, hence our automation does not have alternative and vulnerabilities is there
  • failed login attempts are not logged in (for detection & alerting)
  • there is no "penalty" for wrong password entered few times in a roll, for example, to lock the technical user. So, even less flexibility to have some mitigation in place
  1. then the paradox

Questions:
What other options I have/miss here?
Is there any possibility to "hide" Authentication option in UI for "user/pass" if Local user auth is enabled?

Discussions in this context:
#3208
https://github.com/concourse/rfcs/discussions/50
#5796

@jamieklassen
Copy link
Member

Hi @g-fusion I'm no longer a maintainer of Concourse; best of luck with your automation!

@taylorsilva
Copy link
Member

Hey people, based on Jamie's initial analysis it sounds like before anything happens in Concourse a PR would need to be made to dex to support logging failed login attempts. 90% of the code Concourse has for login/auth is setting up the dex server and configured auth providers. Everything else is handed off to dex. Overall, it sounds like this needs to be a feature request to dex.

@Kump3r
Copy link

Kump3r commented May 22, 2024

Hello @taylorsilva, @jamieklassen. There was a PR towards dex upstream, which was merged in v2.39.0. Since Concourse is using a forк, can I ask if this can be adopted in a new Concourse release as well? As I can see the latest dex in the forked repo is v2.37.0. Thanks for the information!

Edit: I validated the change successfully following this documentation

@taylorsilva
Copy link
Member

Guess someone needs to update our fork then: https://github.com/concourse/dex

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants