Skip to content
This repository has been archived by the owner on Aug 11, 2020. It is now read-only.

Add sudo argument to Cell.all #66

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

yohanboniface
Copy link

A user must have root access to list interfaces, but we don't
want to run python as root, which would be a big security issue.
So this PR allows to only give sudo access to iwlist to the user
running python.

I haven't see any unittests on Cell.all so I haven't added any. If I missed
something, please point me to it :)

cc @ametaireau

A user must have root access to list interfaces, but we don't
want to run python as root, which would be a big security issue.
So this PR allows to only give sudo access to `iwlist` to the user
running python.
@rockymeza
Copy link
Owner

Hi @yohanboniface,

I think this is good.

Don't you need the sudo arg on other calls?

-rocky
2015年5月7日 上午9:44于 "Yohan Boniface" [email protected]写道:

A user must have root access to list interfaces, but we don't
want to run python as root, which would be a big security issue.
So this PR allows to only give sudo access to iwlist to the user
running python.

I haven't see any unittests on Cell.all so I haven't added any. If I
missed
something, please point me to it :)

cc @ametaireau https:/ametaireau

You can view, comment on, or merge this pull request online at:

#66
Commit Summary

  • Add sudo argument to Cell.all

File Changes

Patch Links:


Reply to this email directly or view it on GitHub
#66.

@yohanboniface
Copy link
Author

Good question? :)

Answer is not yet, but certainly in the near future ;)

@splite07, I guess we'll also need sudo to activate a given cell?

@fheslouin
Copy link

Yes you are right!
We will need to have sudo args on save() call, activate() call and probably delete() call

@yohanboniface
Copy link
Author

OK, I'll update my PR this way. :)
@rockymeza adding sudo kwarg to all those method sounds ok for you?

@rockymeza
Copy link
Owner

@yohanboniface, I think it's ok. I don't know what else we would do. I would love to have some unit tests that mock subprocess just to make sure that this is working. (I know it didn't have any before, sorry!)

@rockymeza rockymeza mentioned this pull request Jul 27, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants