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.

Sign up for GitHub

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

Jump to bottom

API Overhaul #8

Merged
merged 5 commits into from Jan 30, 2018
Merged

API Overhaul #8

merged 5 commits into from Jan 30, 2018

Conversation

MggMuggins
Copy link
Contributor

@MggMuggins MggMuggins commented Jan 23, 2018 •
edited

See my commit message for a little more detail. This is big enough that the git diff doesn't really give you a good idea of everything I did, so maybe take a look at the actual source/documentation as well.

As of opening this PR this API is untested. I'll be doing some testing over the next few days, but I don't recommend merging yet. Merging with a new branch would be an option too.

@MggMuggins
API Overhaul
4311ea7 
I rearranged most of the functions into AllGroups and AllUsers, and
adjusted a lot of the API's. I think this is a more sane way of doing
things, but comments are always appreciated.

I removed the Iterators and replaced them with the "get" methods.

Also lots of docs work. I think it should be fairly straightforward to
use this crate now.

@jackpot51 I also fixed the bug where empty passwords will accept
anything.
@MggMuggins MggMuggins closed this Jan 24, 2018
@MggMuggins MggMuggins reopened this Jan 24, 2018
@MggMuggins
Copy link
Contributor Author

MggMuggins commented Jan 24, 2018 •
edited

Whoopsie-daisy...

Notes

  • Implement User::passwd_unset
  • User::verify_passwd should not return Result, only bool. No idea how to accomplish this though: http://bryant.github.io/argon2rs/argon2rs/verifier/struct.Encoded.html (Move parsing the Argon2 hashing session from verify to construction)
  • Agree on a type for uid, gid, euid, and egid (Using usize unless it's otherwise recommended to use u32)
  • Correctly implement file locking
  • Shift User::parse_file to AllUsers::new()
  • Fix dropping to make sure that AllUsers::close or AllGroups::close is called before drop.
  • Get salt for setting password from someplace other than the user of the library (maybe this? 
    //What is used for the passwd binary
let salt = format!("{:X}", OsRng::new().try(&mut stderr).next_u64());
  • Convert get methods on AllUsers and AllGroups to return Option<T>.

@MggMuggins
Polish API
a0513ce 
A full list of accomplishments for this commit can be viewed on my
message on the PR to redox_users. My main difficulty is figuring
out how to save state for AllUsers and AllGroups (Question of wether
to simply write on Drop or add a call for it). 5 other TODO's can be
found.

@goyox86 I'd like to know what you think about all this.
@MggMuggins
Fix login panic with empty encoded field
8bb23d7
@MggMuggins
Copy link
Contributor Author

Alright, I have mostly finished polishing the API. login, id, whoamiand getty work. sudo, su, passwd, useradd, and groupadd do not yet. I'm 99% sure it's something to do with writing the contents of AllUsers/AllGroups to the fs, but until I figure out a sane way to do the API for that I'm not sure how to debug...

@MggMuggins
Bugfix; Fix state-save; Docs
2c6e88e 
Fixed a stupid mistake with User::shell_cmd

Implemented AllUsers::save and AllGroups::save for persisting changes to
the system. @jackpot51 @goyox86 I'd really like to know what you think
about this API. I don't nessesarily like it, but I'm not sure how to
implement something else that's more elegant. :/

Also did a bunch of stuff with docs
@MggMuggins
Massively simplify iterations (fix things)
38babb8
@MggMuggins
Copy link
Contributor Author

The only things that I can find that seem to be wrong at this point is a permissions error while working with su or sudo. Otherwise all the other utils work (including passwd, to it's fullest extent!!!). I'll be gone over the weekend, but I'll open a PR with my (hopefully completed) port of userutils to my new API when I get back.

@jackpot51 jackpot51 self-assigned this Jan 26, 2018
@jackpot51
Copy link
Member

Thanks @MggMuggins, I will review this sometime tomorrow

@goyox86
Copy link
Contributor

goyox86 commented Jan 26, 2018

I will review sometime time today after work too!

@MggMuggins MggMuggins mentioned this pull request Jan 29, 2018
Merged
@jackpot51 jackpot51 merged commit 70b8a69 into redox-os:master Jan 30, 2018
@MggMuggins MggMuggins deleted the break-api branch January 30, 2018 02:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@jackpot51 jackpot51

Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@MggMuggins @jackpot51 @goyox86