Skip to content
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

PLT-27 Implement incoming webhooks. #715

Merged
merged 1 commit into from Sep 22, 2015
Merged

PLT-27 Implement incoming webhooks. #715

merged 1 commit into from Sep 22, 2015

Conversation

jwilander
Copy link
Member

This PR does the following:

  • Creates an api path for webhooks
  • Creates an IncomingWebhook model in a new model/webhook.go file
  • Created a Webhook SQL store (will be used for outgoing webhooks as well)
  • Implements incoming webhook functionality (user POSTs data to an unrestricted URL they generate to post messages to a channel)
  • Added basic UI on the client for adding/removing incoming webhooks inside a new 'Integrations' tab in the account settings
  • Move all user_settings*.jsx components to a new dir
  • Minor style changes on the client

I also edited the config.

@crspeller
Copy link
Member

@jwilander Needs rebase

return
}

c.LogAudit("attempt")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was this intended to be left in? It looks like a debug statement.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Intended. We log an audit when we receive an attempt to create an incoming webhook since once the webhook is created, whoever has it can post as that user. We do audit logs like this in multiple places as well (usually for password related or user modifying related stuff)

@hmhealey hmhealey added the Awaiting Submitter Action Blocked on the author label Sep 21, 2015
@@ -24,7 +24,8 @@
"StorageDirectory": "/mattermost/data/",
"AllowedLoginAttempts": 10,
"DisableEmailSignUp": false,
"EnableOAuthServiceProvider": false
"EnableOAuthServiceProvider": false,
"AllowIncomingWebhooks": false
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should allow webhooks in the preview instances to show them off?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's the plan but I was going to wait until Asaad's UI changes before turning it on by default

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good.

@jwilander jwilander removed the Awaiting Submitter Action Blocked on the author label Sep 21, 2015
@hmhealey
Copy link
Member

Looks good to me, but I'll let Chris pass judgment as the seconder since he had a few issues with it that I missed.

@crspeller
Copy link
Member

+1 Other than the one thing for @coreyhulen

c.Err.StatusCode = http.StatusNotImplemented
return
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add audit attempt/success for delete

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do

@coreyhulen
Copy link
Contributor

@jwilander this is ready correct? +1 from me.

@jwilander
Copy link
Member Author

@coreyhulen yes it's ready

jwilander added a commit that referenced this pull request Sep 22, 2015
PLT-27 Implement incoming webhooks.
@jwilander jwilander merged commit ac7918c into master Sep 22, 2015
@jwilander jwilander deleted the plt-27 branch September 22, 2015 19:47
@jwilander jwilander removed the 2: Dev Review Requires review by a developer label Sep 22, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants