Add the ability to save Git login temporarily
#1099
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Adjust git push, pull, fetch, clone handlers, allowing `auth` and `cache_credentials` to be passed in in requests. - Allow `credential.helper` to pass through `git config` handler's filters.
Allow users to configure the value of `credential_helper` to be used in the credentials caching logic.
Implemennt methods that ensure `credential.helper` in `git config` to be used for credentials caching.
Extend git command logics (push, pull, fetch, clone) to support credentials caching when `auth` is provided.
Add tests to test the newly-implemented credentials caching logic for `git push`, `git pull`, `git fetch` and `git clone`.
Extend `Git.IAuth` and `CredentialsBox` for credentials caching. There is a checkbox for saving Git credentials temporarily. Stylings are also made for `CredentialsBox`. `jp-RedirectForm` does not work well with `input` checkboxes.
Let each of these methods also include `auth?.cacheCredentials` to POST to the backend.
Add extra class members to take care of the fetch poll when credentials are required.
Add method `fetch()` that is similar to how `push()` or `pull()` works. `_fetchRemotes()` now calls `fetch()`, and if credentials are required, blocks further calls and signals a credentials requirement status change.
`_fetchBlocked` is not needed.
This dialog now supports `fetch` operation, and upon successful authentication, unblocks the fetch poll by changing the credentials requirement state to `false`.
Also let `showGitOperationDialog()` accept `IGitExtension` `model` instead of the concrete class `GitExtension`.
The component now has a boolean for Git credentials requirement. Also add a callback connected to the model to update that boolean.
Inject the model to the component and reuse `showGitOperationDialog()` for the `fetch` operation.
Add a notification dot to the component so that it looks like the `push` (when the local branch is ahead) button and `pull` button (when the local branch is behind) on the top toolbar in the Git panel.
This method spawns a Git credential cache daemon so that the caching mechanism can work.
Let these methods check if a cache daemon is required, and invoke `ensure_git_credential_cache_daemon()` if on Linux, or else respond with an error message.
Let them return an error response if the credential helper cannot be ensured.
- Use `assert_awaited_` instead of `assert_called_` in some places. - Use `assert_has_awaits` instead of `assert_has_calls` in some places. - Use name `mock_execute` instead of `mock_authentication`. - Mock `sys.platform` as `"linux"`. - Make sure the `ensure_git_credential_cache_daemon()` is called accordingly.
This is so that the front-end can recognize this response with `IResultWithMessage`.
|
|
Add argument `cache_credentials` to assertions.
This is for compatibility with Python 3.7's `unittest`.
fcollonval
requested changes
Mar 24, 2022
There was a problem hiding this comment.
Thanks @quachtridat and @ZeshanFayyaz
I did a first review of the code.
jupyterlab/pull/1099#pullrequestreview-919970442
fcollonval
requested changes
Mar 30, 2022
There was a problem hiding this comment.
Thanks @quachtridat for applying the suggestions.
Would you mind fixing the CI?
I added two minor suggestions more.
Yes I am working on fixing the CI! |
- jupyterlab/pull/1099#pullrequestreview-926143691 - Remove redundant tests - Adjust tests to match changed method signatures - Add a missing `self.config()` call when ensuring Git credential helper
Also fix a minor typo.
fcollonval
approved these changes
Apr 4, 2022
fcollonval
added a commit
that referenced
this pull request
Apr 20, 2022
* Adjust git command handlers - Adjust git push, pull, fetch, clone handlers, allowing `auth` and `cache_credentials` to be passed in in requests. - Allow `credential.helper` to pass through `git config` handler's filters. * Add `credential_helper` to server settings Allow users to configure the value of `credential_helper` to be used in the credentials caching logic. * Implement logic for setting git credential helper Implemennt methods that ensure `credential.helper` in `git config` to be used for credentials caching. * Extend git commands to support credentials caching Extend git command logics (push, pull, fetch, clone) to support credentials caching when `auth` is provided. * Add tests for the new credentials caching logic Add tests to test the newly-implemented credentials caching logic for `git push`, `git pull`, `git fetch` and `git clone`. * Extend `Git.IAuth` and `CredentialsBox` Extend `Git.IAuth` and `CredentialsBox` for credentials caching. There is a checkbox for saving Git credentials temporarily. Stylings are also made for `CredentialsBox`. `jp-RedirectForm` does not work well with `input` checkboxes. * Extend `clone()`, `push()`, `pull()` in model Let each of these methods also include `auth?.cacheCredentials` to POST to the backend. * Extend model for `_fetchRemotes()` Add extra class members to take care of the fetch poll when credentials are required. * Implement `fetch()` Add method `fetch()` that is similar to how `push()` or `pull()` works. `_fetchRemotes()` now calls `fetch()`, and if credentials are required, blocks further calls and signals a credentials requirement status change. * Remove a redundant member `_fetchBlocked` is not needed. * Extend `showGitOperationDialog()` This dialog now supports `fetch` operation, and upon successful authentication, unblocks the fetch poll by changing the credentials requirement state to `false`. * Match the `IGitExtension` with `push()` in model Also let `showGitOperationDialog()` accept `IGitExtension` `model` instead of the concrete class `GitExtension`. * Extend `StatusWidget` The component now has a boolean for Git credentials requirement. Also add a callback connected to the model to update that boolean. * Reuse `showGitOperationDialog()` in `StatusWidget` Inject the model to the component and reuse `showGitOperationDialog()` for the `fetch` operation. * Add a notification dot to `StatusWidget` Add a notification dot to the component so that it looks like the `push` (when the local branch is ahead) button and `pull` button (when the local branch is behind) on the top toolbar in the Git panel. * Implement `ensure_git_credential_cache_daemon()` This method spawns a Git credential cache daemon so that the caching mechanism can work. * Extend `check/ensure_credential_helper()` Let these methods check if a cache daemon is required, and invoke `ensure_git_credential_cache_daemon()` if on Linux, or else respond with an error message. * Refactor `push/pull/fetch/clone` Let them return an error response if the credential helper cannot be ensured. * Refactor and adjust tests to adopt new changes - Use `assert_awaited_` instead of `assert_called_` in some places. - Use `assert_has_awaits` instead of `assert_has_calls` in some places. - Use name `mock_execute` instead of `mock_authentication`. - Mock `sys.platform` as `"linux"`. - Make sure the `ensure_git_credential_cache_daemon()` is called accordingly. * Include `"message"` in `fetch()`'s response This is so that the front-end can recognize this response with `IResultWithMessage`. * Lint files with ESLint * Adjust `test_handlers.py` to adopt new changes Add argument `cache_credentials` to assertions. * Revert `await` back to `call` This is for compatibility with Python 3.7's `unittest`. * Adopt PR review suggestions /pull/1099#pullrequestreview-919970442 * Fix CI - /pull/1099#pullrequestreview-926143691 - Remove redundant tests - Adjust tests to match changed method signatures - Add a missing `self.config()` call when ensuring Git credential helper * Reformat `model.ts` with Prettier Also fix a minor typo. * Add new icons for commit comparison * Extend `Git.diff()` to accept two commit hashes * Implement `IGitExtension.diff()` in `src/model.ts` * Implement new React component `CommitComparisonBox` This component displays a comparison between two commits. * Add two buttons to `PastCommitNode` These buttons allow selecting commits for comparison. * Implement interface `ICommitComparison` This interface is used for comparison results. * Move the function `openDiff()` out to `util.ts` This will also be used by the commit comparison logic that will exist in `src/components/GitPanel.tsx` later. * Extend `HistorySideBar` for commit comparison * Extend `GitPanel` to allow commit comparison The comparison status is invalidated when the branch status gets refreshed. * Fix linting and format problems * Mock new props in `HistorySideBar` test file * Implemented naming convention changes * Add docstring for `ICommitComparisonBoxProps` * Add a missing `trans` prop for CommitComparisonBox * Remove the `linesplit` check * Slight increase of the border width when comparing The left border width for commits being compared is 6px now. * Remove commit comparison state redundancy Address the commit comparison state redundancy in `GitPanel`. /pull/1108#discussion_r851098378 * Apply suggestions from code review Co-authored-by: Zeshan Fayyaz <[email protected]> Co-authored-by: Frédéric Collonval <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR fixes #659 by implementing the ability that lets JupyterLab-Git users cache their login credentials. By default, the information is saved temporarily for 1 hour, and this can be configured (the unit of time is seconds). This feature is readily available for any one of the following Git operations:
Within the cache period, any of the aforementioned Git operations should not ask for the user's credentials again and should be able to work using the cached credentials.
Details
Frontend
Git.IAuthis extended with an optionalcache_credentials: boolean.Backend
push,pull,fetch,cloneeach accepts aPOSTrequest that includesauthandauthincludescache_credentials(optional/nullable/None-able). These are used to indicate what the user provided from the frontend.git configis used to make a decision here. If the user wishes to have their credentials temporarily saved, and ingit config,credential.helperis not set, thencredential.helperwill be set to whatever that is configured in the server settings. Otherwise, no action is taken. After that, in either case, ifcredential.helperiscache(i.e., Git's built-in credential caching mechanism), JupyterLab will attempt to spawn a daemon process that takes care of the caching work. In a brief amount of time after the daemon has been summoned, credentials given to Git will be picked up by that daemon.References
https://git-scm.com/docs/git-credential-cache: A
--timeoutargument can be added to specify how long (in seconds) credentials should be cached.https://git-scm.com/docs/git-credential-cache--daemon
Demos
An orange dot appears on top of the Git icon in the status bar, indicating that it needs Git credentials.
Clicking the Git icon in the status bar should make this prompt pop-up show up. This also applies for doing clones, pushes and pulls.
There should be no colored dot while the cached information is alive and is valid.
The dot will show up again when the cached information does not exist (anymore) or is no longer valid. Also in this case, doing clones, pushes or pulls would require the user to input their Git credentials again.
Notes
This PR might need more tests.
Linked issues
#659