-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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(intel-security): Add com-intel-security-cordova-plugin Support #1256
Conversation
Applications will not bomb if the plugin is not installed or user is running in browser.
/** | ||
* @hidden | ||
*/ | ||
export class IntelSecurityData { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can add another @Plugin()
tag here with a different pluginRef
and no repo
property. You can also make it injectable if you think that's a better way to do it.
If you make it Injectable. Then the class above should be empty and only used for documentation purposes.
Apply this fix to the class below as well.
* @param id {string} Storage resource identifier. | ||
* @returns {Promise<any>} Returns a Promise that resolves with the instanceID of the created data instance, or rejects with an error. | ||
*/ | ||
createFromData(data: string): Promise<any> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After making the changes I mentioned above, you can remove the if (!window.intel) ...
check and add the regular @Cordova
decorator.
Apply this fix to all methods below.
Changes made, thank you! The only thing I can't figure out is now the console is logging |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good now. Just one last change mentioned above and then this will be good to go.
export class IntelSecurityData { | ||
|
||
/** | ||
* This creates a new instance of secure data using plain-text data. | ||
* @param id {string} Storage resource identifier. | ||
* @returns {Promise<any>} Returns a Promise that resolves with the instanceID of the created data instance, or rejects with an error. | ||
*/ | ||
@Cordova() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since now we have the Cordova decorator, there is no need to write any functionally. We can just have the keyword return
in there.
If the real method takes an object as a parameter instead of a string, then we need to change the parameter type in the method definition/docs.
@@ -4,32 +4,6 @@ import { Injectable } from '@angular/core'; | |||
declare var window: any; | |||
declare var intel: any; | |||
|
|||
export interface IntelSecurityDataOptions { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I liked the interface better than having the params listed below.
this.storage = new IntelSecurityStorage(); | ||
this.data = new IntelSecurityData(); | ||
|
||
try { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove these 5 lines as they are not needed.
pluginName: 'IntelSecurity', | ||
plugin: 'com-intel-security-cordova-plugin', | ||
pluginRef: 'intel.security.secureData', | ||
repo: '' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove the repo
property completely.
* .catch((error: any) => console.log(error)); | ||
* | ||
* ``` | ||
*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add the following
* @classes
* IntelSecurityData
* IntelSecurityStorage
* @param [options.webOwners] {String[]} List of trusted web domains. | ||
* @returns {Promise<any>} Returns a Promise that resolves with the instanceID of the created data instance, or rejects with an error. | ||
*/ | ||
@Cordova() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replace all @Cordova()
decorators with @Cordova({ otherPromise: true })
.
I just realized their plugin returns a promise instead of using callbacks. This decorator config will convert their Q
promise to a regular ES6 Promise.
Thanks @ihadeed -- I'm working on the changes now. Some "author notes" from me: :-P
Anyway, changes coming soon once I can verify they work in a sample Ionic project. Thank you for the help and guidance through this! |
@ihadeed -- I'm running into this issue now after changing the
|
@ehorodyski just pushed a commit to the master branch to fix this issue. |
This PR adds support for the com-intel-security-cordova-plugin.