Skip to content

Commit

Permalink
misc zoom cleanup (#847)
Browse files Browse the repository at this point in the history
* zoom client id isn't sensitive

* document that zoom renders this stuff in clear in web consoles

* some zoom troubleshooting insights

* add a bunch of trims to AccountCredentialsGrantTokenRequestBuilder

* change placeholder warning to info; was confusing people in logs

* drop unused local
  • Loading branch information
eschultink authored Dec 12, 2024
1 parent 4795933 commit 8b2bd80
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 12 deletions.
18 changes: 18 additions & 0 deletions docs/sources/zoom/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -81,3 +81,21 @@ sufficient, but as of May 2024 are no longer available for newly created Zoom ap
Once the scopes are added, click on `Done` and then `Continue`.

6. Activate the app

## Troubleshooting

### Zoom API Error : 400 invalid client

`{"reason":"Invalid client_id or client_secret","error":"invalid_client"}`

Causes:
- extra chars in Client ID; or incorrect Client ID

Confirm that the `Client ID` and `Client Secret` are correctly set in your secret store solution (AWS Parameter Store, Secrets Manager, or GCP Secret Manager).

### Zoom API Error : 400 Bad Request

`{"reason":"Bad Request","error":"invalid_request"}`

Causes:
- extra chars in Account ID; or incorrect Account ID
2 changes: 0 additions & 2 deletions infra/modules/aws-ssm-secrets/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@ locals {

externally_managed_secrets = { for k, spec in var.secrets : k => spec if !(spec.value_managed_by_tf) }
terraform_managed_secrets = { for k, spec in var.secrets : k => spec if spec.value_managed_by_tf }

tf_management_description_appendix = "Value managed by a Terraform configuration; changes outside Terraform may be overwritten by subsequent 'terraform apply' runs"
}

# see: https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/ssm_parameter
Expand Down
4 changes: 2 additions & 2 deletions infra/modules/worklytics-connector-specs/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -913,14 +913,14 @@ EOT
{
name : "CLIENT_ID"
writable : false
sensitive : false
sensitive : false # zoom renders in clear in console
value_managed_by_tf : false
description : "Client ID of the Zoom 'Server-to-Server' OAuth App used by the Connector to retrieve Zoom data. Value should be obtained from your Zoom admin."
},
{
name : "ACCOUNT_ID"
writable : false
sensitive : true
sensitive : false # zoom renders in clear in console
value_managed_by_tf : false
description : "Account ID of the Zoom tenant from which the Connector will retrieve Zoom data. Value should be obtained from your Zoom admin."
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import com.google.api.client.http.UrlEncodedContent;
import lombok.Getter;
import lombok.NoArgsConstructor;
import org.apache.commons.lang3.StringUtils;

import javax.inject.Inject;
import java.nio.charset.StandardCharsets;
Expand Down Expand Up @@ -60,23 +61,26 @@ public enum ConfigProperty implements ConfigService.ConfigProperty {

@Override
public HttpContent buildPayload() {

//trim to avoid copy-paste errors
//q : check for non-printable chars or anything like that
String accountId = StringUtils.trim(config.getConfigPropertyAsOptional(ConfigProperty.ACCOUNT_ID)
.orElseGet(() -> secretStore.getConfigPropertyOrError(ConfigProperty.ACCOUNT_ID)));

// The documentation doesn't say anything to use POST data, but passes everything in the URL
// Tested manually and, for the moment, it is accepted as POST data
Map<String, String> data = new TreeMap<>();
data.put(PARAM_GRANT_TYPE, getGrantType());
//q: move to config? cost-benefit, mainly
data.put(PARAM_ACCOUNT_ID,
config.getConfigPropertyAsOptional(ConfigProperty.ACCOUNT_ID)
.orElseGet(() -> secretStore.getConfigPropertyOrError(ConfigProperty.ACCOUNT_ID)));
data.put(PARAM_ACCOUNT_ID, accountId);
return new UrlEncodedContent(data);
}

@Override
public void addHeaders(HttpHeaders httpHeaders) {
String clientId = config.getConfigPropertyAsOptional(ConfigProperty.CLIENT_ID)
.orElseGet(() -> secretStore.getConfigPropertyOrError(ConfigProperty.CLIENT_ID));
String clientId = StringUtils.trim(config.getConfigPropertyAsOptional(ConfigProperty.CLIENT_ID)
.orElseGet(() -> secretStore.getConfigPropertyOrError(ConfigProperty.CLIENT_ID)));

String clientSecret = secretStore.getConfigPropertyOrError(ConfigProperty.CLIENT_SECRET);
String clientSecret = StringUtils.trim(secretStore.getConfigPropertyOrError(ConfigProperty.CLIENT_SECRET));
String token = Base64.getEncoder()
.encodeToString(String.join(":", clientId, clientSecret).getBytes(StandardCharsets.UTF_8));
httpHeaders.setAuthorization("Basic " + token);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ <T> Optional<T> getConfigPropertyAsOptional(ConfigProperty property, Function<Ge

Optional<T> r;
if (Objects.equals(parameterResponse.parameter().value(), PLACEHOLDER_VALUE)) {
log.warning("Found placeholder value for " + paramName + "; this is either a misconfiguration, or a value that proxy itself should later fill.");
log.info("Found placeholder value for " + paramName + "; this is either a misconfiguration, or a value that proxy itself should later fill.");
r = Optional.empty();
} else {
if (envVarsConfig.isDevelopment()) {
Expand Down

0 comments on commit 8b2bd80

Please sign in to comment.