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

feat(lib): Updated error types #362

Merged
merged 7 commits into from
Oct 15, 2024
Merged

Conversation

dmihalcik-virtru
Copy link
Member

  • Untyped Error objects indicate a likely bug in the library itself
    • The messages should be prefixed internal:
    • Samples include bad resource management or missing values in fields that should be const.
  • TdfError should be the root for all errors an application might theoretically screen for to find out if something is wrong in their application that might be caused by TDF or this library. Includes a novel code field to allow tracking based on a unique(?) error code
  • ConfigurationError should be able to be fixed by updating the application code.
  • InvalidFileError indicates that a file is likely tampered with or corrupt, although for some errors this may also indicate something is wrong with the user KAS. There are several subtypes when there may be changes to the configuration or user settings that could potentially fix the issue.
    • DecryptError may indicate that the key is incorrect; this could be caused by using a remote or CKS key is out of date, indicating a failure on the server side, but at the moment we have not implemented this.
    • IntegrityError indicates that the segment or global hash is incorrect. This could indicate a file was generated with a deprecated library that uses a different hash calculation.
    • UnsafeUrlError indicates that one or more required Key Access Objects refers to a remote KAS that is not in the allowlist. You can manually check the URL and add it to the allowlist in the client constructor if is a supported KAS.
  • NetworkError indicates a network connectivity error (e.g. during rewrap or key lookup), or a 5xx error on a service
  • UnauthenticatedError indicates that the Bearer token or a required DPoP was not attached to a request. This is often fixable with a mix of IdP/OAuth configuration changes and changes to the application or by adding custom middleware or some combination of all these.
  • PermissionDeniedError indicates that a service (rewrap or public key) has denied access, either due to traditional login (bearer token insufficient scope is one possibility) or due to ABAC (client entity does not have sufficient attributes for policy)
  • UnsupportedFeatureError indicates that an enum in the file or a KAS requirement is not met, e.g. KAS uses an unsupported EC curve, or the TDF file embeds such a curve; could indicate the file was generated with a newer, experimental, or deprecated/removed feature.

@dmihalcik-virtru dmihalcik-virtru requested a review from a team as a code owner October 14, 2024 17:59
elizabethhealy
elizabethhealy previously approved these changes Oct 14, 2024
lib/src/access.ts Show resolved Hide resolved
lib/src/errors.ts Show resolved Hide resolved
- Untyped `Error` objects indicate a likely bug in the library itself
  - The messages should be prefixed `internal: `
  - Samples include bad resource management or missing values in fields that should be const.
- `TdfError` should be the root for all errors an application might theoretically screen for to find out if something is wrong in their application that might be caused by TDF or this library. Includes a novel `code` field to allow tracking based on a unique(?) error code
- `ConfigurationError` should be able to be fixed by updating the application code.
- `InvalidFileError` indicates that a file is likely tampered with or corrupt, although for some errors this may also indicate something is wrong with the user KAS. There are several subtypes when there may be changes to the configuration or user settings that could potentially fix the issue.
   - `DecryptError` may indicate that the key is incorrect; this could be caused by using a remote or CKS key is out of date, indicating a failure on the server side, but at the moment we have not implemented this.
   - `IntegrityError` indicates that the segment or global hash is incorrect. This could indicate a file was generated with a deprecated library that uses a different hash calculation.
   - `UnsafeUrlError` indicates that one or more required Key Access Objects refers to a remote KAS that is not in the allowlist. You can manually check the URL and add it to the allowlist in the client constructor if is a supported KAS.
- `NetworkError` indicates a network connectivity error (e.g. during rewrap or key lookup), or a 5xx error on a service
- `UnauthenticatedError` indicates that the Bearer token or a required DPoP was not attached to a request. This is often fixable with a mix of IdP/OAuth configuration changes and changes to the application or by adding custom middleware or some combination of all these.
- `PermissionDeniedError` indicates that a service (rewrap or public key) has denied access, either due to traditional login (bearer token insufficient scope is one possibility) or due to ABAC (client entity does not have sufficient attributes for policy)
- `UnsupportedFeatureError` indicates that an enum in the file or a KAS requirement is not met, e.g. KAS uses an unsupported EC curve, or the TDF file embeds such a curve; could indicate the file was generated with a newer, experimental, or deprecated/removed feature.
elizabethhealy
elizabethhealy previously approved these changes Oct 15, 2024
@@ -102,7 +102,9 @@ export class SplitKey {
: typeof metadata === 'string'
? metadata
: () => {
throw new Error();
throw new Error(
Copy link
Member

Choose a reason for hiding this comment

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

base error?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes this maybe should be invalidFileError, now that I look closer

Copy link
Member Author

Choose a reason for hiding this comment

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

oh wait nevermind this is during write. Configuration error probably

if (!algorithm) {
throw new Error('Uninitialized cipher type');
// Hard coded as part of the cipher object. This should not be reachable.
throw new Error('internal: uninitialized cipher type');
Copy link
Member

Choose a reason for hiding this comment

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

base error?

Copy link
Member Author

Choose a reason for hiding this comment

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

While I think this is most likely an internal error, it could conceivably also be a configuration error if the client is highly customized (a la secure share)

pflynn-virtru
pflynn-virtru previously approved these changes Oct 15, 2024
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
40.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarCloud

@dmihalcik-virtru dmihalcik-virtru merged commit 7fb29c5 into main Oct 15, 2024
11 of 12 checks passed
@dmihalcik-virtru dmihalcik-virtru deleted the feature/better-errors branch October 15, 2024 15:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants