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

Add functionality for auto-revoking auth tokens #224

Merged
merged 2 commits into from Aug 23, 2017
Merged

Add functionality for auto-revoking auth tokens #224

merged 2 commits into from Aug 23, 2017

Conversation

jmoldow
Copy link
Contributor

@jmoldow jmoldow commented Aug 22, 2017

Auth objects can now be closed, which prevents them from being
used to request new tokens. This will also revoke any existing
tokens. Also introduces a closing() context manager method,
which will auto-close the auth object on exit. Clients can use
this to make sure that their tokens don't live longer than they
need them for.

Auth objects can now be closed, which prevents them from being
used to request new tokens. This will also revoke any existing
tokens. Also introduces a `closing()` context manager method,
which will auto-close the auth object on exit. Clients can use
this to make sure that their tokens don't live longer than they
need them for.
@boxcla
Copy link

boxcla commented Aug 22, 2017

Verified that @jmoldow has signed the CLA. Thanks for the pull request!

Copy link
Contributor

@Jeff-Meadows Jeff-Meadows left a comment

Choose a reason for hiding this comment

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

Should we add an auto-revoking client that uses this auth behavior?


def _check_closed(self):
if self.closed:
raise ValueError("operation on a closed auth object")
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure ValueError is correct here. I'd raise RuntimeError or a custom Exception

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ValueError is what the standard library uses for similar purposes:

jmoldow Lib ((detached from v3.6.2)) $ git grep -C9 "if .*closed.*[:]" | grep -v asyncio | grep -v pyio | grep -v "test" | grep --color=always "  raise "
_compression.py-14-            raise ValueError("I/O operation on closed file")
_compression.py-18-            raise io.UnsupportedOperation("File not open for reading")
_compression.py-22-            raise io.UnsupportedOperation("File not open for writing")
chunk.py-95-            raise ValueError("I/O operation on closed file")
chunk.py-105-            raise ValueError("I/O operation on closed file")
chunk.py-107-            raise OSError("cannot seek")
chunk.py-113-            raise RuntimeError
chunk.py-119-            raise ValueError("I/O operation on closed file")
chunk.py-129-            raise ValueError("I/O operation on closed file")
chunk.py-153-            raise ValueError("I/O operation on closed file")
idlelib/run.py-339-            raise ValueError("write to closed file")
idlelib/run.py-342-                raise TypeError('must be str, not ' + type(s).__name__)
idlelib/run.py-359-            raise ValueError("read from closed file")
idlelib/run.py-363-            raise TypeError('must be int, not ' + type(size).__name__)
idlelib/run.py-382-            raise ValueError("read from closed file")
idlelib/run.py-386-            raise TypeError('must be int, not ' + type(size).__name__)
pathlib.py-1011-        raise ValueError("I/O operation on closed path")
pathlib.py-1072-            raise ValueError("Unacceptable pattern: {!r}".format(pattern))
pathlib.py-1191-            raise TypeError('data must be str, not %s' %
socket.py-615-            raise ValueError("I/O operation on closed socket.")
socket.py-622-            raise ValueError("I/O operation on closed socket.")
socket.py-629-            raise ValueError("I/O operation on closed socket.")
subprocess.py-188-            raise ValueError("already closed")
tarfile.py-408-                raise CompressionError("unknown compression type %r" % comptype)
tarfile.py-2365-            raise OSError("%s is closed" % self.__class__.__name__)
tarfile.py-2367-            raise OSError("bad operation for mode %r" % self.mode)
tempfile.py-678-            raise ValueError("Cannot enter context with closed file")
urllib/response.py-30-            raise ValueError("I/O operation on closed file")
zipfile.py-985-            raise ValueError('I/O operation on closed file.')

@jmoldow
Copy link
Contributor Author

jmoldow commented Aug 23, 2017

@Jeff-Meadows see my comment in #175. I experimented with adding a __del__ for OAuth2, but I didn't like the behavior, so I removed it.

For the client, we could think about making it into a context manager, which invoke's auth.closing().

@jmoldow jmoldow merged commit a764630 into master Aug 23, 2017
@jmoldow jmoldow deleted the auth_close branch August 23, 2017 19:53
potrebic pushed a commit that referenced this pull request Sep 13, 2017
Auth objects can now be closed, which prevents them from being
used to request new tokens. This will also revoke any existing
tokens. Also introduces a `closing()` context manager method,
which will auto-close the auth object on exit. Clients can use
this to make sure that their tokens don't live longer than they
need them for.
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.

None yet

3 participants