Setting password can result in loss of custom data

[bdemers]

Originally reported by: @brussellc5 in: okta/okta-sdk-java#117

With the Okta version of the Stormpath SDK, we're getting these same results (loss of custom profile attributes) in the following scenarios:

To reset a forgotten password: Application.resetPassword(token, password)
To update a password: resolve Account from servlet request, Account.setPassword(), Account.save()

In each case I'd view that as a problem in that I wouldn't expect either of those methods to update anything other than a password.

[george-hawkins-work]

OK - I've investigated thoroughly and documented what happens below.

When a user resets their password this results in a PUT /api/v1/users/:id call to the backend that includes the user's profile as well as their updated credentials. However the profile does not contain the user's custom data.

So unsurprisingly we lose all custom data associated with the user as the Okta update user documentation states:

All profile properties must be specified when updating a user’s profile with a PUT method. Any property not specified in the request is deleted.

Important: Don’t use PUT method for partial updates.

After a lot of investigation the sequence of steps that results in custom data not being included in the profile data is fairly clear - the issue seems to stem from the switch over from the Stormpath to Okta backed.

Previously when account data for a user was retrieved special case handling for custom data kicked-in in the AbstractExtendableInstanceResource superclass constructor of DefaultAccount:

if (properties != null && properties.containsKey(CUSTOM_DATA.getName())) { ...
    ...
    CustomData customData = ...
    properties.put(CUSTOM_DATA.getName(), customData);
}

The important thing here is that the custom data gets created as a concrete implementation of CustomData. However now the properties retrieved from Okta do not contain a field called customData instead the properties look like this and custom data is handled later:

properties = {
    id=00ubq22vnuPxyDy9Q0h7,
    status=ACTIVE,
    created=2017-08-21T09:09:48.000Z,
    activated=2017-08-21T09:09:49.000Z,
    statusChanged=2017-08-21T09:09:49.000Z,
    lastLogin=2017-08-22T12:42:47.000Z,
    lastUpdated=2017-08-24T11:53:29.000Z,
    passwordChanged=2017-08-24T11:01:59.000Z,
    profile={
        lastName=Hawkins,
        secondEmail=null,
        mobilePhone=null,
        emailVerificationStatus=UNKNOWN,
        email=george.hawkins+test@amanaadvisors.com,
        userId=613d81a9-bbc1-461c-8277-650aa76368f6,
        login=george.hawkins+test@amanaadvisors.com,
        firstName=George
    },
    credentials={
        password={},
        recovery_question={
            question=stormpathMigrationRecoveryAnswer
        },
        provider={
            type=OKTA,
            name=OKTA
        }
    },
    _links={
        suspend={href=https://dev-603022.oktapreview.com/api/v1/users/00ubq22vnuPxyDy9Q0h7/lifecycle/suspend, method=POST},
        resetPassword={href=https://dev-603022.oktapreview.com/api/v1/users/00ubq22vnuPxyDy9Q0h7/lifecycle/reset_password, method=POST},
        expirePassword={href=https://dev-603022.oktapreview.com/api/v1/users/00ubq22vnuPxyDy9Q0h7/lifecycle/expire_password, method=POST},
        forgotPassword={href=https://dev-603022.oktapreview.com/api/v1/users/00ubq22vnuPxyDy9Q0h7/credentials/forgot_password, method=POST},
        self={href=https://dev-603022.oktapreview.com/api/v1/users/00ubq22vnuPxyDy9Q0h7},
        changeRecoveryQuestion={href=https://dev-603022.oktapreview.com/api/v1/users/00ubq22vnuPxyDy9Q0h7/credentials/change_recovery_question, method=POST},
        deactivate={href=https://dev-603022.oktapreview.com/api/v1/users/00ubq22vnuPxyDy9Q0h7/lifecycle/deactivate, method=POST},
        changePassword={href=https://dev-603022.oktapreview.com/api/v1/users/00ubq22vnuPxyDy9Q0h7/credentials/change_password, method=POST}
    }
}

Our custom data, in this case userId above, is just mixed in with the other data in profile so the old check for a customData field never fires. Now it's only later than custom data is constructed by OktaUserAccountConverter:

Daemon Thread [http-nio-8080-exec-1] (Suspended (breakpoint at line 327 in OktaUserAccountConverter))    
    OktaUserAccountConverter.trimMap(Map<String,Object>, String...) line: 327    
    OktaUserAccountConverter.toAccount(Map<String,Object>, String) line: 154    
    DefaultAccount.setProperties(Map<String,Object>) line: 151    
    DefaultAccount(AbstractExtendableInstanceResource).<init>(InternalDataStore, Map<String,Object>) line: 54    
    DefaultAccount.<init>(InternalDataStore, Map<String,Object>) line: 145    

The trimMap method creates a LinkedHashMap and so we end up with OktaUserAccountConverter inserting this as the STORMPATH_CUSTOM_DATA property rather than some concrete instance of CustomData.

This has fatal consequences later when the ChangePasswordController logic tries to save the user with an updated password:

Daemon Thread [http-nio-8080-exec-10] (Suspended)    
    DefaultResourceConverter.toMapValue(AbstractResource, String, Object, boolean) line: 93    
    DefaultResourceConverter.toMap(AbstractResource, boolean, boolean) line: 78    
    DefaultResourceConverter.convert(AbstractResource) line: 60    
    DefaultDataStore.save(String, T, HttpHeaders, Class<? extends R>, QueryString, boolean) line: 454    
    DefaultDataStore.save(T) line: 417    
    OktaApplication.resetPassword(String, String) line: 275    
    ChangePasswordController.onValidSubmit(HttpServletRequest, HttpServletResponse, Form) line: 242    
    ChangePasswordController(FormController).doPost(HttpServletRequest, HttpServletResponse) line: 233    

In the call DefaultResourceConverter.toMapValue(AbstractResource, String, Object, boolean) there's special case logic for custom data:

if (resource instanceof CustomData) {
    //no sanitization: CustomData resources retain their values as-is:
    return value;
}

As our custom data is an instance of LinkedHashMap rather than CustomData this logic never kicks in and instead our custom data is reduced down to just a map containing a href value by com.stormpath.sdk.impl.resource.ReferenceFactory.createReference(String, Map).

And then this single entry is itself later removed by the trimMap functionality invoked by the toUser functionality:

Daemon Thread [http-nio-8080-exec-3] (Suspended)    
    LinkedHashMap<K,V>(HashMap<K,V>).remove(Object) line: 798    
    OktaUserAccountConverter.trimMap(Map<String,Object>, String...) line: 324    
    OktaUserAccountConverter.toUser(Map<String,Object>) line: 215    
    DefaultResourceConverter.toMap(AbstractResource, boolean, boolean) line: 84    
    DefaultResourceConverter.convert(AbstractResource) line: 60    
    DefaultDataStore.save(String, T, HttpHeaders, Class<? extends R>, QueryString, boolean) line: 454    
    DefaultDataStore.save(T) line: 417    
    OktaApplication.resetPassword(String, String) line: 275    
    ChangePasswordController.onValidSubmit(HttpServletRequest, HttpServletResponse, Form) line: 242    
    ChangePasswordController(FormController).doPost(HttpServletRequest, HttpServletResponse) line: 233    

So the custom data map ends up totally empty and as a result toUser ends up returning:

userMap = {
    created=2017-08-21T09:09:48.000Z,
    lastUpdated=2017-08-24T11:53:29.000Z,
    credentials={
        password={value=FooBar12}
    },
    profile={
        login=george.hawkins+test@amanaadvisors.com,
        email=george.hawkins+test@amanaadvisors.com,
        firstName=George,
        lastName=Hawkins,
        emailVerificationStatus=UNKNOWN
    }
}

So credentials looks OK but profile contains no custom data.

This ends up being packed into the request that is sent to the Okta backend by DefaultDataStore.save(T):

Daemon Thread [http-nio-8080-exec-2] (Suspended)    
    DefaultFilterChain.filter(ResourceDataRequest) line: 52    
    DefaultDataStore.save(String, T, HttpHeaders, Class<? extends R>, QueryString, boolean) line: 512    
    DefaultDataStore.save(T) line: 417    
    OktaApplication.resetPassword(String, String) line: 275    
    ChangePasswordController.onValidSubmit(HttpServletRequest, HttpServletResponse, Form) line: 242    
    ChangePasswordController(FormController).doPost(HttpServletRequest, HttpServletResponse) line: 233    

And results in the following HTTP request:

http-outgoing-5 >> "PUT /api/v1/users/00ubq22vnuPxyDy9Q0h7 HTTP/1.1[\r][\n]"
http-outgoing-5 >> "Host: dev-603022.oktapreview.com[\r][\n]"
http-outgoing-5 >> "Accept-Encoding: gzip[\r][\n]"
http-outgoing-5 >> "Accept: application/json[\r][\n]"
http-outgoing-5 >> "User-Agent: stormpath-spring-security/2.0.0-okta stormpath-servlet-java/2.0.0-okta stormpath-spring-webmvc/2.0.0-okta stormpath-spring-boot-starter/2.0.0-okta stormpath-sdk-java/2.0.0-okta stormpath-oktagration/2.0.0-okta spring-security/4.2.1.RELEASE spring/4.3.5.RELEASE spring-boot/1.5.1.RELEASE tomcat/8.5 java/1.8.0_45 Mac OS X/10.10.5[\r][\n]"
http-outgoing-5 >> "Content-Type: application/json[\r][\n]"
http-outgoing-5 >> "Authorization: SSWS 00oAGhrCvRurKLnBppf0VnLfcde6fi-5Fpf-qlYvpQ[\r][\n]"
http-outgoing-5 >> "Content-Length: 313[\r][\n]"
http-outgoing-5 >> "Connection: Keep-Alive[\r][\n]"
http-outgoing-5 >> "Expect: 100-continue[\r][\n]"
http-outgoing-5 >> "[\r][\n]"
http-outgoing-5 << "HTTP/1.1 100 Continue[\r][\n]"
http-outgoing-5 << "[\r][\n]"
http-outgoing-5 >> "{"created":"2017-08-21T09:09:48.000Z","lastUpdated":"2017-08-24T11:53:29.000Z","credentials":{"password":{"value":"FooBar12"}},"profile":{"login":"george.hawkins+test@amanaadvisors.com","email":"george.hawkins+test@amanaadvisors.com","firstName":"George","lastName":"Hawkins","emailVerificationStatus":"UNKNOWN"}}"

See that profile included in the HTTP PUT body does not contain any custom data. And unsurprisingly we lose all custom data associated with the user 😞

This is a really serious issue for us - we use custom data to associate the user in Okta with the data we maintain for them in our own DB. So every time a user resets their password we lose this data.

[george-hawkins-work]

I confirm that after applying pull request #1356 locally I now see a POST rather than a PUT when updating credentials - and that this results in profile fields that aren't explicitly included surviving the request rather than being deleted as happened previously when POST was used.

POST /api/v1/users/00ubq22vnuPxyDy9Q0h7 HTTP/1.1
Host: dev-603022.oktapreview.com
Accept-Encoding: gzip
Accept: application/json
Content-Type: application/json
Authorization: SSWS XX-XX-XX
Content-Length: 313
Connection: Keep-Alive

{"created":"2017-08-21T09:09:48.000Z","lastUpdated":"2017-08-24T11:53:29.000Z","credentials":{"password":{"value":"FooBar12"}},"profile":{"login":"george.hawkins+test@amanaadvisors.com","email":"george.hawkins+test@amanaadvisors.com","firstName":"George","lastName":"Hawkins","emailVerificationStatus":"UNKNOWN"}}

Hmm... I suspect I should have been censoring at the very least the Authorization headers in previous comments.