Page MenuHomePhabricator

Correctly handle case where no ssh keys are found
ClosedPublic

Authored by Firehed on Nov 30 2013, 8:13 AM.
Tags
None
Referenced Files
F14416032: D7675.diff
Tue, Dec 24, 8:23 PM
Unknown Object (File)
Mon, Dec 23, 6:36 AM
Unknown Object (File)
Mon, Dec 23, 6:36 AM
Unknown Object (File)
Mon, Dec 23, 6:36 AM
Unknown Object (File)
Mon, Dec 23, 6:24 AM
Unknown Object (File)
Tue, Dec 17, 3:34 AM
Unknown Object (File)
Fri, Dec 13, 6:43 PM
Unknown Object (File)
Thu, Dec 12, 9:27 AM

Details

Reviewers
epriestley
Group Reviewers
Blessed Reviewers
Commits
Restricted Diffusion Commit
rP45c3a605ee7b: Correctly handle case where no ssh keys are found
Summary

If no ssh keys are in phabricator, bin/ssh-auth errors with undefined $lines. This fixes that case and explicitly tells the user no rows were found.

Test Plan

Ran bin/ssh-auth before and after change with no ssh keys in the system. Error goes away after change.

Diff Detail

Branch
handle_no_ssh_entries
Lint
Lint Errors
SeverityLocationCodeMessage
Errorsrc/applications/auth/controller/PhabricatorEmailTokenController.php:54PHL1Unknown Symbol
Errorsrc/applications/calendar/view/AphrontCalendarMonthView.php:90PHL1Unknown Symbol
Errorsrc/applications/conpherence/controller/ConpherenceNotificationPanelController.php:75PHL1Unknown Symbol
Errorsrc/applications/differential/landing/DifferentialLandingToGitHub.php:85PHL1Unknown Symbol
Errorsrc/applications/differential/render/DifferentialChangesetHTMLRenderer.php:253PHL1Unknown Symbol
Errorsrc/applications/differential/view/DifferentialAddCommentView.php:189PHL1Unknown Symbol
Errorsrc/applications/differential/view/DifferentialInlineCommentView.php:233PHL1Unknown Symbol
Errorsrc/applications/differential/view/DifferentialLocalCommitsView.php:131PHL1Unknown Symbol
Errorsrc/applications/differential/view/DifferentialRevisionCommentView.php:90PHL1Unknown Symbol
Errorsrc/applications/differential/view/DifferentialRevisionUpdateHistoryView.php:205PHL1Unknown Symbol
Errorsrc/applications/diffusion/controller/DiffusionCommitController.php:747PHL1Unknown Symbol
Errorsrc/applications/diffusion/view/DiffusionCommentView.php:142PHL1Unknown Symbol
Errorsrc/applications/feed/builder/PhabricatorFeedBuilder.php:51PHL1Unknown Symbol
Errorsrc/applications/feed/controller/PhabricatorFeedDetailController.php:30PHL1Unknown Symbol
Errorsrc/applications/feed/controller/PhabricatorFeedListController.php:35PHL1Unknown Symbol
Errorsrc/applications/feed/controller/PhabricatorFeedPublicStreamController.php:28PHL1Unknown Symbol
Errorsrc/applications/herald/controller/HeraldTranscriptController.php:371PHL1Unknown Symbol
Errorsrc/applications/maniphest/controller/ManiphestTaskDescriptionPreviewController.php:21PHL1Unknown Symbol
Errorsrc/applications/maniphest/controller/ManiphestTaskDetailController.php:352PHL1Unknown Symbol
Errorsrc/applications/notification/controller/PhabricatorNotificationListController.php:48PHL1Unknown Symbol
Errorsrc/applications/notification/controller/PhabricatorNotificationPanelController.php:23PHL1Unknown Symbol
Errorsrc/applications/people/controller/PhabricatorPeopleProfileController.php:130PHL1Unknown Symbol
Errorsrc/applications/phame/controller/post/PhamePostEditController.php:146PHL1Unknown Symbol
Errorsrc/applications/phame/controller/post/PhamePostPreviewController.php:26PHL1Unknown Symbol
Errorsrc/applications/phriction/storage/PhrictionContent.php:78PHL1Unknown Symbol
Unit
No Test Coverage

Event Timeline

I don't think we should exit 0 in this case, as sshd will attempt to read the output and authorize the user using the list of keys found on stdout. Although it's probably hard to escalate "No keys found.\n" into an access breach, it seems less confusing to prevent sshd from getting that far.

So, let's either emit this to stderr and exit 0 with empty stdout, or emit it to stdout and exit non-0.

This should also be pht()'d.

Firehed updated this revision to Unknown Object (????).Nov 30 2013, 5:45 PM
  • exit(1) on error

(That lint can be cleared by updating libphutil/. If you have several copies on disk, the copy you need to update is shown in the first few lines of arc list --trace. In some cases, it may not be the same copy which Phabricator uses at runtime. The symbol was added in rPHUff78852, so whichever copy is being loaded is older than that.)

Closed by commit rP45c3a605ee7b (authored by @Firehed, committed by @epriestley).