Constants should always be only assigned once. The logical OR assignment
of a constant is a relic of the past before we used zeitwerk for
autoloading and had bugs where a file could be loaded twice resulting in
constant redefinition warnings.
Our old group SMTP SSL option was a checkbox,
but this was not ideal because there are actually
3 different ways SSL can be used when sending
SMTP:
* None
* SSL/TLS
* STARTTLS
We got around this before with specific overrides
for Gmail, but it's not flexible enough and now people
want to use other providers. It's best to be clear,
though it is a technical detail. We provide a way
to test the SMTP settings before saving them so there
should be little chance of messing this up.
This commit also converts GroupEmailSettings to a glimmer
component.
If, for whatever reasons, the user's locale is "blank" and an admin is accepting their group membership request, there will be an error because we're generating posts with the locale of recipient.
In order to fix this, we now use the `user.effective_locale` which takes care of multiple things, including returning the default locale when the user's locale is blank.
Internal ref - t/132347
Instead of creating two separate Topics when a user (1) requests to join a group and (2) gets accepted in, this makes the acceptance message into a Post under the origin group request Topic.
In 07ecbb5a3b we ensure the mentions in a group's activity page worked properly but we missed adding proper support for infinite loading.
The client is using the `before` parameter instead of the `before_post_id` to do the pagination.
This adds support for `before` as well as some tests to ensure it doesn't regress.
I also added tests to the group's activity posts as well since those were missing.
Finally I deleted some unused code (`group.messages_for`) which is not used anymore.
Context - https://meta.discourse.org/t/-/308044/9
* UX: add sorting params to groups table plugin outlet
* FEATURE: allow sorting group members by custom field via API
---------
Co-authored-by: Jean Perez <jmperez127@gmail.com>
* FIX: respect creation date when paginating group activity posts
There are scenarios where the chronological order of posts doesn't match the order of their IDs. For instance, when moving the first post from one topic or PM to another, a new post (with a higher ID) will be created, but it will retain the original creation time.
This PR changes the group activity page and endpoint to paginate posts using created_at instead of relying on ID ordering.
What is the problem here?
In multiple controllers, we are accepting a `limit` params but do not
impose any upper bound on the values being accepted. Without an upper
bound, we may be allowing arbituary users from generating DB queries
which may end up exhausing the resources on the server.
What is the fix here?
A new `fetch_limit_from_params` helper method is introduced in
`ApplicationController` that can be used by controller actions to safely
get the limit from the params as a default limit and maximum limit has
to be set. When an invalid limit params is encountered, the server will
respond with the 400 response code.
This commit adds modifiers that allow plugins to change how categories and groups are prefetched into the application and listed in the respective controllers.
Possible use cases:
- prevent some categories/groups from being prefetched when the application loads for performance reasons.
- prevent some categories/groups from being listed in their respective index pages.
* FIX: Redirect if Discourse-Xhr-Redirect is present
`handleRedirect` was passed an wrong argument type (a string) instead of
a jqXHR object and missed the fields checked in condition, thus always
evaluating to `false`.
* FIX: Add `errors` field if group update confirmation
An explicit confirmation about the effect of the group update is
required if the default notification level changes. Previously, if the
confirmation was missing the API endpoint failed silently returning
a 200 response code and a `user_count` field. This change ensures that
a proper error code is returned (422), a descriptive error message and
the additional information in the `user_count` field.
This commit also refactors the API endpoint to use the
`Discourse-Xhr-Redirect` header to redirect the user if the group is
no longer visible.
Sorting group members worked always kept the group owners at the top of
the list. This commit keeps the group owners at the top of the list only
when no order exists.
This commit allows group SMTP emails to be sent with a
different from email address that has been set up as an
alias in the email provider. Emails from the alias will
be grouped correctly using Message-IDs in the mail client,
and replies to the alias go into the correct group inbox.
This commit adds a check that runs regularly as per
2d68e5d942 which tests the
credentials of groups with SMTP or IMAP enabled. If any issues
are found with those credentials a high priority problem is added to the
admin dashboard.
This commit also formats the admin dashboard differently if
there are high priority problems, bringing them to the top of
the list and highlighting them.
The problem will be cleared if the issue is fixed before the next
problem check, or if the group's settings are updated with a valid
credential.
This commit introduces a new site setting "google_oauth2_hd_groups". If enabled, group information will be fetched from Google during authentication, and stored in the Discourse database. These 'associated groups' can be connected to a Discourse group via the "Membership" tab of the group preferences UI.
The majority of the implementation is generic, so we will be able to add support to more authentication methods in the near future.
https://meta.discourse.org/t/managing-group-membership-via-authentication/175950
Previously, a group's `default_notification_level` change will only affect the users added after it.
Co-authored-by: Alan Guo Xiang Tan <gxtan1990@gmail.com>
We shouldn't be checking if a user is allowed to do an action in the logger. We should be checking it just before we perform the action. In fact, guardians in the logger can make things even worse in case of a security bug. Let's say we forgot to check user's permissions before performing some action, but we still have a call to the guardian in the logger. In this case, a user would perform the action anyway, and this action wouldn't even be logged!
I've checked all cases and I confirm that we're safe to delete this calls from the logger.
I've added two calls to guardians in admin/user_controller. We didn't have security bugs there, because regular users can't access admin/... routes at all. But it's good to have calls to guardian in these methods anyway, neighboring methods have them.
* Copy remove_member to new `leave` method
* Remove unneeded code from the leave method
* Rearrange the leave method
* Remove unneeded code from the remove_member method
* Add tests
* Implement on the client side
* Copy the add_members method to the new join method
* Remove unneeded code from the join method
* Rearrange the join method
* Remove unneeded stuff from the add_members method
* Extract add_user_to_group method
* Implement of the client side
* Tests
* Doesn't inline users.uniq
* Return promise from join.then()
* Remove unnecessary begin and end
* Revert "Return promise from join.then()"
This reverts commit bda84d8d
* Remove variable already_in_group
It used to require SiteSetting.min_trust_level_to_allow_invite to
invite a user to a group, even if the user existed and the inviter was
a group owner.
This overhauls the user interface for the group email settings management, aiming to make it a lot easier to test the settings entered and confirm they are correct before proceeding. We do this by forcing the user to test the settings before they can be saved to the database. It also includes some quality of life improvements around setting up IMAP and SMTP for our first supported provider, GMail. This PR does not remove the old group email config, that will come in a subsequent PR. This is related to https://meta.discourse.org/t/imap-support-for-group-inboxes/160588 so read that if you would like more backstory.
### UI
Both site settings of `enable_imap` and `enable_smtp` must be true to test this. You must enable SMTP first to enable IMAP.
You can prefill the SMTP settings with GMail configuration. To proceed with saving these settings you must test them, which is handled by the EmailSettingsValidator.
If there is an issue with the configuration or credentials a meaningful error message should be shown.
IMAP settings must also be validated when IMAP is enabled, before saving.
When saving IMAP, we fetch the mailboxes for that account and populate them. This mailbox must be selected and saved for IMAP to work (the feature acts as though it is disabled until the mailbox is selected and saved):
### Database & Backend
This adds several columns to the Groups table. The purpose of this change is to make it much more explicit that SMTP/IMAP is enabled for a group, rather than relying on settings not being null. Also included is an UPDATE query to backfill these columns. These columns are automatically filled when updating the group.
For GMail, we now filter the mailboxes returned. This is so users cannot use a mailbox like Sent or Trash for syncing, which would generally be disastrous.
There is a new group endpoint for testing email settings. This may be useful in the future for other places in our UI, at which point it can be extracted to a more generic endpoint or module to be included.
It used to allow adding email addresses to a group even if invites were
disabled for the site. This does not allow user to input email address
if they cannot invite.
The second thing this commit improves is the message that is displayed
to the user when they hit the invite rate limit.
This PR allows invitations to be used when the DiscourseConnect SSO is enabled for a site (`enable_discourse_connect`) and local logins are disabled. Previously invites could not be accepted with SSO enabled simply because we did not have the code paths to handle that logic.
The invitation methods that are supported include:
* Inviting people to groups via email address
* Inviting people to topics via email address
* Using invitation links generated by the Invite Users UI in the /my/invited/pending route
The flow works like this:
1. User visits an invite URL
2. The normal invitation validations (redemptions/expiry) happen at that point
3. We store the invite key in a secure session
4. The user clicks "Accept Invitation and Continue" (see below)
5. The user is redirected to /session/sso then to the SSO provider URL then back to /session/sso_login
6. We retrieve the invite based on the invite key in secure session. We revalidate the invitation. We show an error to the user if it is not valid. An additional check here for invites with an email specified is to check the SSO email matches the invite email
7. If the invite is OK we create the user via the normal SSO methods
8. We redeem the invite and activate the user. We clear the invite key in secure session.
9. If the invite had a topic we redirect the user there, otherwise we redirect to /
Note that we decided for SSO-based invites the `must_approve_users` site setting is ignored, because the invite is a form of pre-approval, and because regular non-staff users cannot send out email invites or generally invite to the forum in this case.
Also deletes some group invite checks as per https://github.com/discourse/discourse/pull/12353
The user interface has been reorganized to show email and link invites
in the same screen. Staff has more control over creating and updating
invites. Bulk invite has also been improved with better explanations.
On the server side, many code paths for email and link invites have
been merged to avoid duplicated logic. The API returns better responses
with more appropriate HTTP status codes.
The 'Discourse SSO' protocol is being rebranded to DiscourseConnect. This should help to reduce confusion when 'SSO' is used in the generic sense.
This commit aims to:
- Rename `sso_` site settings. DiscourseConnect specific ones are prefixed `discourse_connect_`. Generic settings are prefixed `auth_`
- Add (server-side-only) backwards compatibility for the old setting names, with deprecation notices
- Copy `site_settings` database records to the new names
- Rename relevant translation keys
- Update relevant translations
This commit does **not** aim to:
- Rename any Ruby classes or methods. This might be done in a future commit
- Change any URLs. This would break existing integrations
- Make any changes to the protocol. This would break existing integrations
- Change any functionality. Further normalization across DiscourseConnect and other auth methods will be done separately
The risks are:
- There is no backwards compatibility for site settings on the client-side. Accessing auth-related site settings in Javascript is fairly rare, and an error on the client side would not be security-critical.
- If a plugin is monkey-patching parts of the auth process, changes to locale keys could cause broken error messages. This should also be unlikely. The old site setting names remain functional, so security-related overrides will remain working.
A follow-up commit will be made with a post-deploy migration to delete the old `site_settings` rows.
