Discourse_api sync_sso custom_fields results in "custom.custom.<field>" in payload

Contribute Bug

AI-generated summary

Issue with custom fields in Discourse API SSO sync

vkozyrev reported an issue with the sync_sso method in the Discourse API, where custom fields are being prefixed with custom. twice, resulting in custom.custom.field_1 in the payload. This is due to the sync_sso method passing the custom. prefix to sso.custom_fields.

vkozyrev proposed a possible solution by modifying the sync_sso method to accept both DiscourseApi::SingleSignOn instances and hashes. Alternatively, they suggested defining params with a nested custom_fields hash.

However, blake pointed out that a fix for this issue was already made last month, and can be found in this commit.

1

https://github.com/discourse/discourse_api/pull/190

DiscourseApi::SingleSignOn expects custom attributes to be assigned like

sso.custom_fields['field_1'] = 'value'

# which results in `custom.field_1` key in payload

DiscourseApi::API::SSO.sync_sso() expects params to have

{ 'custom.field_1' => 'value` }

But it passes in the custom. prefix to sso.custom_fields
This results in the payload to having

custom.custom.field_1

Also added some test overkill for sync_sso

1 Like
2

This would have been a lot simpler imo, but don’t wanna break existing apps :grimacing:. Leave SSO instance generation up to the user.

# frozen_string_literal: true
module DiscourseApi
  module API
    module SSO
      def sync_sso(sso)
        post("/admin/users/sync_sso", sso.payload)
      end
    end
  end
end

Maybe tomorrow I’ll do something like (but neater)

# frozen_string_literal: true
module DiscourseApi
  module API
    module SSO
      def sync_sso(params)
        if params.instance_of?(DiscourseApi::SingleSignOn)
          post("/admin/users/sync_sso", sso.payload) 
        elsif params.instance_of?(Hash)
          .... same as current
        end
      end
    end
  end
end

Thoughts? My app will ideally have a single function that generates DiscourseApi::SingleSignOn, and I’d then be able to pass it to sso_login or sync_sso

Or params can be defined like

{
  sso_secret: ...
  name: ...
  custom_fields: {
    field_1: ...
  }
}

Which would also break existing apps.

2 Likes
3

A fix was made for this last month:

https://github.com/discourse/discourse_api/commit/9587ed754a3f7164d66defe339e48a174bb2f1e7

2 Likes