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

AP/WIFI check password validity for encryption type #760

Closed
johnnyman727 opened this issue Jun 3, 2016 · 10 comments · Fixed by #792
Closed

AP/WIFI check password validity for encryption type #760

johnnyman727 opened this issue Jun 3, 2016 · 10 comments · Fixed by #792

Comments

@johnnyman727
Copy link
Contributor

OpenWRT will silently fail if you try to set the password on an access point (or wifi connection but that's less of an issue) to something that is not valid for the encryption type. For example, I tried to create an access point with the encryption type of psk2 and the password of stones but that password was not long enough to be valid. The CLI reported that it was set correctly but the network was never created.

Ideally, we should be able to check for password-encryption validity before we attempt to set it in OpenWRT. The CLI should WARN the user if it is invalid and abort.

@HipsterBrown
Copy link
Contributor

HipsterBrown commented Jun 3, 2016

This is a great addition @johnnyman727. I have also been looking into this OpenWRT recipe to fallback to ap/wifi only, if one of those configs fail -> https://forum.openwrt.org/viewtopic.php?pid=309131#p309131

There is another method of doing that through Step 3 of this OpenWRT recipe -> http://wiki.openwrt.org/doc/recipes/ap_sta

@HipsterBrown
Copy link
Contributor

HipsterBrown commented Jun 5, 2016

For anyone who wants to claim this issue:

  • claim it by commenting below: @laiedwin
  • add AP password validation by checking out lib/controller.js. If a password exists, validate it based on the passed in security option. wpa and wpa2 should be removed from the securityOptions array as well, since we don't have support for WPA Enterprise right now.
  • add tests for the various password validations
  • open a PR

You can find out more about the password validation for the various security options, check out:

Feel free to ping me for review or questions.

@smilin-desperado
Copy link

Just saw this on @yourfirstpr. I'd love to give it a try

@HipsterBrown
Copy link
Contributor

@smilin-desperado Awesome, it's all yours! Let me know if you need any help or review along the way. You can join the Slack team here -> https://tessel-slack.herokuapp.com/ or just mention me in a comment in this issue. :)

@smilin-desperado
Copy link

@HipsterBrown Been too busy to work on this recently and I'm not sure when I will have more time so I will have to 'unclaim' the issue so someone else can have a go at it.

@HipsterBrown
Copy link
Contributor

@smilin-desperado Thanks for giving it a shot anyway and letting me know about unclaiming it. Please reach out if you ever want to contribute again in the future. 😄

@laiedwin
Copy link
Contributor

laiedwin commented Jul 9, 2016

I'd love to take a stab at this!

@HipsterBrown
Copy link
Contributor

@laiedwin Awesome, thanks! It's all yours! Feel free to reach out if you have any questions or want me to review.

@laiedwin
Copy link
Contributor

laiedwin commented Jul 10, 2016

Hi @HipsterBrown , I've opened PR #792 for this.

Few things I'm not entirely sure about:

  • Is using var preferred over the es6-style let/const?
  • Are my tests in the right place?
  • Could the naming of my test methods be improved somewhat?

Feel free to let me know if there's any changes I should make!

@HipsterBrown
Copy link
Contributor

@laiedwin Ok I'll review that today and add feedback to that PR.

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

Successfully merging a pull request may close this issue.

4 participants