Skip to content
This repository was archived by the owner on May 15, 2024. It is now read-only.

GH-564 Handle Key Unwrap Exception#609

Merged
mattleibow merged 4 commits intoxamarin:dev/api-fixfrom
jamesmontemagno:dev/bug-fix-564
Nov 5, 2018
Merged

GH-564 Handle Key Unwrap Exception#609
mattleibow merged 4 commits intoxamarin:dev/api-fixfrom
jamesmontemagno:dev/bug-fix-564

Conversation

@jamesmontemagno
Copy link
Copy Markdown
Collaborator

@jamesmontemagno jamesmontemagno commented Nov 5, 2018

Description of Change

Need to properly handle issues when key can't be unwrapped.

Bugs Fixed

Provide links to issues here. Ensure that a GitHub issue was created for your feature or bug fix before sending PR.

Behavioral Changes

If the key can not be unwrapped then tear down the entire secure storage stack. This needs to be documented. It should not happen ideally ever.

PR Checklist

  • Has tests (if omitted, state reason in description)
  • Has samples (if omitted, state reason in description)
  • Rebased on top of master at time of PR
  • Changes adhere to coding standard
  • Updated documentation (see walkthrough)

@jamesmontemagno jamesmontemagno added the awaiting-review This PR needs to have a set of eyes on it label Nov 5, 2018
@jamesmontemagno jamesmontemagno added this to the 1.0.0 milestone Nov 5, 2018
@jamesmontemagno jamesmontemagno requested a review from Redth November 5, 2018 20:05
@ghost
Copy link
Copy Markdown

ghost commented Nov 5, 2018

✅ Validation status: passed

File Status Preview URL Details
DeviceTests/DeviceTests.Shared/SecureStorage_Tests.cs ✅Succeeded
Xamarin.Essentials/SecureStorage/SecureStorage.android.cs ✅Succeeded

For more details, please refer to the build report.

Note: If you changed an existing file name or deleted a file, broken links in other files to the deleted or renamed file are listed only in the full build report.

@jamesmontemagno jamesmontemagno added DO-NOT-MERGE Don't merge it.... don't do it! and removed awaiting-review This PR needs to have a set of eyes on it labels Nov 5, 2018
@ghost
Copy link
Copy Markdown

ghost commented Nov 5, 2018

✅ Validation status: passed

File Status Preview URL Details
DeviceTests/DeviceTests.Shared/SecureStorage_Tests.cs ✅Succeeded
Xamarin.Essentials/SecureStorage/SecureStorage.android.cs ✅Succeeded

For more details, please refer to the build report.

Note: If you changed an existing file name or deleted a file, broken links in other files to the deleted or renamed file are listed only in the full build report.

Redth
Redth reviewed Nov 5, 2018
View reviewed changes
Comment thread Xamarin.Essentials/SecureStorage/SecureStorage.android.cs Outdated
@jamesmontemagno jamesmontemagno added awaiting-review This PR needs to have a set of eyes on it and removed DO-NOT-MERGE Don't merge it.... don't do it! labels Nov 5, 2018
@ghost
Copy link
Copy Markdown

ghost commented Nov 5, 2018

✅ Validation status: passed

File Status Preview URL Details
DeviceTests/DeviceTests.Shared/SecureStorage_Tests.cs ✅Succeeded
Xamarin.Essentials/SecureStorage/SecureStorage.android.cs ✅Succeeded

For more details, please refer to the build report.

Note: If you changed an existing file name or deleted a file, broken links in other files to the deleted or renamed file are listed only in the full build report.

@ghost
Copy link
Copy Markdown

ghost commented Nov 5, 2018

❌ Validation status: errors

File Status Preview URL Details
❌Error Details

  • [Error] File:../../Tests/UnitConverters_Tests.cs not existed.
  • [Error] File:../../Xamarin.Essentials/Types/UnitConverters.shared.cs not existed.
  • [Error] File:Xamarin.Essentials/UnitConverters.xml not existed.
  • [Error] Some error ocurred while parsing changelist file: W:\j2t2-log-c9fe35cc\change-list-with-redirection.mapped.tsv.

For more details, please refer to the build report.

Note: If you changed an existing file name or deleted a file, broken links in other files to the deleted or renamed file are listed only in the full build report.

@jamesmontemagno
Copy link
Copy Markdown
Collaborator Author

jamesmontemagno commented Nov 5, 2018

I think this is fine, openpublishing is confused.

@mattleibow mattleibow merged commit 8d0712a into xamarin:dev/api-fix Nov 5, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers

@Redth Redth Redth left review comments

@mattleibow mattleibow mattleibow approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

awaiting-review This PR needs to have a set of eyes on it

Projects

None yet

Milestone

1.0.0

Development

Successfully merging this pull request may close these issues.

3 participants

@jamesmontemagno @Redth @mattleibow