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

App password command #24317

Merged
merged 1 commit into from
Jan 6, 2021
Merged

Conversation

SMillerDev
Copy link
Contributor

Add a command to generate app passwords. This will make it a lot easier for app developers to do full end-to-end tests and hopefully it'll also benefit some users.

I don't know much about the server code, so please point to other things you guys need in this PR.

@SMillerDev
Copy link
Contributor Author

@nextcloud/server-triage maybe?
crickets

core/Command/User/AddAppPassword.php Outdated Show resolved Hide resolved
@skjnldsv skjnldsv added the 2. developing Work in progress label Dec 22, 2020
@skjnldsv
Copy link
Member

@SMillerDev please set development status label and milestone :)
That helps us knowing what to do with this pr.

@SMillerDev SMillerDev added this to the Nextcloud 21 milestone Dec 22, 2020
@SMillerDev SMillerDev added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Dec 22, 2020
@SMillerDev
Copy link
Contributor Author

It's that documented somewhere? Without the comment I would never have known.

@rullzer rullzer mentioned this pull request Dec 23, 2020
39 tasks
@rullzer rullzer force-pushed the app_password_command branch from d243a2a to eeee12a Compare December 23, 2020 11:48
Copy link
Member

@rullzer rullzer left a comment

Choose a reason for hiding this comment

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

Fine by me for a first version!

@SMillerDev
Copy link
Contributor Author

Thanks for the help @rullzer . Do I need to do anything here?

@ChristophWurst
Copy link
Member

It's that documented somewhere? Without the comment I would never have known.

https://docs.nextcloud.com/server/20/developer_manual/getting_started/codingguidelines.html#labels

$output->writeln('<info>The password is not validated so what you provide is what gets recorded in the token</info>');


$token = $this->random->generate(72, ISecureRandom::CHAR_UPPER.ISecureRandom::CHAR_LOWER.ISecureRandom::CHAR_DIGITS);
Copy link
Member

Choose a reason for hiding this comment

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

can we make the magic number 72 a constant somewhere so we use the same length in all code paths?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure why it's 72 to begin with, @rullzer any comment on this?

Copy link
Member

Choose a reason for hiding this comment

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

72 was done because of the bcrypt limit of 72 bytes

@rullzer rullzer mentioned this pull request Dec 28, 2020
39 tasks
@rullzer rullzer force-pushed the app_password_command branch from 41962b1 to 6bb7078 Compare December 30, 2020 12:31
@rullzer rullzer mentioned this pull request Jan 4, 2021
5 tasks
core/Command/User/AddAppPassword.php Outdated Show resolved Hide resolved
core/register_command.php Outdated Show resolved Hide resolved
Signed-off-by: Sean Molenaar <sean@seanmolenaar.eu>
@SMillerDev SMillerDev force-pushed the app_password_command branch from 92a8099 to 40595f3 Compare January 5, 2021 12:05
@SMillerDev
Copy link
Contributor Author

Pushed the fix and resolved a deprecation notice in the command registration

Copy link
Member

@MorrisJobke MorrisJobke left a comment

Choose a reason for hiding this comment

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

Tested and works 👍

@rullzer rullzer merged commit ad3735b into nextcloud:master Jan 6, 2021
@welcome
Copy link

welcome bot commented Jan 6, 2021

Thanks for your first pull request and welcome to the community! Feel free to keep them coming! If you are looking for issues to tackle then have a look at this selection: https://github.com/nextcloud/server/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22

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

Successfully merging this pull request may close these issues.

5 participants